diff --git a/CHANGELOG.md b/CHANGELOG.md index e00e9db3f0..271435a81f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -688,6 +688,8 @@ - [One can define lazy atom fields][6151] - [Replace IOContexts with Execution Environment and generic Context][6171] - [Vector.sort handles incomparable types][5998] +- [Removing need for asynchronous thread to execute ResourceManager + finalizers][6335] [3227]: https://github.com/enso-org/enso/pull/3227 [3248]: https://github.com/enso-org/enso/pull/3248 @@ -794,6 +796,7 @@ [6151]: https://github.com/enso-org/enso/pull/6151 [6171]: https://github.com/enso-org/enso/pull/6171 [5998]: https://github.com/enso-org/enso/pull/5998 +[6335]: https://github.com/enso-org/enso/pull/6335 # Enso 2.0.0-alpha.18 (2021-10-12) diff --git a/engine/interpreter-dsl-test/src/test/java/org/enso/interpreter/dsl/test/InliningBuiltinsOutNode.java b/engine/interpreter-dsl-test/src/test/java/org/enso/interpreter/dsl/test/InliningBuiltinsOutNode.java index d24ac1ebb9..4d3f950062 100644 --- a/engine/interpreter-dsl-test/src/test/java/org/enso/interpreter/dsl/test/InliningBuiltinsOutNode.java +++ b/engine/interpreter-dsl-test/src/test/java/org/enso/interpreter/dsl/test/InliningBuiltinsOutNode.java @@ -9,7 +9,7 @@ import org.junit.Assert; final class InliningBuiltinsOutNode extends Node { long execute(VirtualFrame frame, long a, long b) { - Assert.assertNotNull("VirtualFrame is always provided " + frame); + Assert.assertNotNull("VirtualFrame is always provided", frame); return a + b; } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/ResourceManager.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/ResourceManager.java index 8527214a55..df7d6ba7c4 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/ResourceManager.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/ResourceManager.java @@ -1,6 +1,8 @@ package org.enso.interpreter.runtime; import com.oracle.truffle.api.CompilerDirectives; +import com.oracle.truffle.api.ThreadLocalAction; +import com.oracle.truffle.api.TruffleSafepoint; import com.oracle.truffle.api.interop.InteropLibrary; import org.enso.interpreter.runtime.data.ManagedResource; @@ -9,8 +11,10 @@ import java.lang.ref.Reference; import java.lang.ref.ReferenceQueue; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.Future; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; /** Allows the context to attach garbage collection hooks on the removal of certain objects. */ public class ResourceManager { @@ -59,7 +63,7 @@ public class ResourceManager { return; } it.getParkedCount().decrementAndGet(); - tryFinalize(it); + scheduleFinalizationAtSafepoint(it); } /** @@ -75,7 +79,7 @@ public class ResourceManager { return; } // Unconditional finalization – user controls the resource manually. - it.doFinalize(context); + it.finalizeNow(context); } /** @@ -89,7 +93,7 @@ public class ResourceManager { items.remove(resource.getPhantomReference()); } - private void tryFinalize(Item it) { + private void scheduleFinalizationAtSafepoint(Item it) { if (it.isFlaggedForFinalization().get()) { if (it.getParkedCount().get() == 0) { // We already know that isFlaggedForFinalization was true at some @@ -101,8 +105,21 @@ public class ResourceManager { // no further attempts are made. boolean continueFinalizing = it.isFlaggedForFinalization().compareAndSet(true, false); if (continueFinalizing) { - it.doFinalize(context); - items.remove(it.reference); + var futureToCancel = new AtomicReference>(null); + var performFinalizeNow = + new ThreadLocalAction(false, false, true) { + @Override + protected void perform(ThreadLocalAction.Access access) { + var tmp = futureToCancel.getAndSet(null); + if (tmp == null) { + return; + } + tmp.cancel(false); + it.finalizeNow(context); + items.remove(it.reference); + } + }; + futureToCancel.set(context.getEnvironment().submitThreadLocal(null, performFinalizeNow)); } } } @@ -124,7 +141,7 @@ public class ResourceManager { } if (workerThread == null || !workerThread.isAlive()) { worker.setKilled(false); - workerThread = context.getEnvironment().createThread(worker); + workerThread = context.getEnvironment().createSystemThread(worker); workerThread.start(); } ManagedResource resource = new ManagedResource(object); @@ -158,7 +175,7 @@ public class ResourceManager { Item it = items.remove(key); if (it != null) { // Finalize unconditionally – all other threads are dead by now. - it.doFinalize(context); + it.finalizeNow(context); } } } @@ -181,7 +198,7 @@ public class ResourceManager { continue; } it.isFlaggedForFinalization().set(true); - tryFinalize(it); + scheduleFinalizationAtSafepoint(it); } if (killed) { return; @@ -229,18 +246,16 @@ public class ResourceManager { } /** - * Unconditionally performs the finalization action of this resource. + * Performs the finalization action of this resource right now. The thread must be inside of a + * context. * * @param context current execution context */ - public void doFinalize(EnsoContext context) { - Object p = context.getThreadManager().enter(); + public void finalizeNow(EnsoContext context) { try { InteropLibrary.getUncached(finalizer).execute(finalizer, underlying); } catch (Exception e) { context.getErr().println("Exception in finalizer: " + e.getMessage()); - } finally { - context.getThreadManager().leave(p); } } diff --git a/engine/runtime/src/test/scala/org/enso/interpreter/test/InterpreterTest.scala b/engine/runtime/src/test/scala/org/enso/interpreter/test/InterpreterTest.scala index d7bb17d2dd..7d96aef4f3 100644 --- a/engine/runtime/src/test/scala/org/enso/interpreter/test/InterpreterTest.scala +++ b/engine/runtime/src/test/scala/org/enso/interpreter/test/InterpreterTest.scala @@ -113,6 +113,7 @@ class InterpreterContext( .newBuilder(LanguageInfo.ID) .allowExperimentalOptions(true) .allowAllAccess(true) + .allowCreateThread(false) .out(output) .err(err) .option(RuntimeOptions.LOG_LEVEL, "WARNING") diff --git a/engine/runtime/src/test/scala/org/enso/interpreter/test/semantic/RuntimeManagementTest.scala b/engine/runtime/src/test/scala/org/enso/interpreter/test/semantic/RuntimeManagementTest.scala index 7b03f019fa..524d5e77a1 100644 --- a/engine/runtime/src/test/scala/org/enso/interpreter/test/semantic/RuntimeManagementTest.scala +++ b/engine/runtime/src/test/scala/org/enso/interpreter/test/semantic/RuntimeManagementTest.scala @@ -81,7 +81,11 @@ class RuntimeManagementTest extends InterpreterTest { totalOut = consumeOut while (totalOut.length < expect && round < 500) { round = round + 1 - if (round % 10 == 0) forceGC(); + if (round % 10 == 0) { + forceGC(); + } + val res = eval("main a b = a * b").execute(7, 6) + assertResult(42)(res.asInt) Thread.sleep(100) totalOut ++= consumeOut }