Removing need for asynchronous thread to execute ResourceManager finalizers

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)
 

engine/interpreter-dsl-test/src/test/java/org/enso/interpreter/dsl/test/InliningBuiltinsOutNode.java

@@ -9,7 +9,7 @@
 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;
   }
 

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.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 void unpark(ManagedResource resource) {
       return;
     }
     it.getParkedCount().decrementAndGet();
-    tryFinalize(it);
+    scheduleFinalizationAtSafepoint(it);
   }
 
   /**
@@ -75,7 +79,7 @@ public void close(ManagedResource resource) {
       return;
     }
     // Unconditional finalization – user controls the resource manually.
-    it.doFinalize(context);
+    it.finalizeNow(context);
   }
 
   /**
@@ -89,7 +93,7 @@ public void take(ManagedResource resource) {
     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 @@ private void tryFinalize(Item it) {
         // 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<Future<Void>>(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 ManagedResource register(Object object, Object function) {
     }
     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 void shutdown() {
       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 void run() {
               continue;
             }
             it.isFlaggedForFinalization().set(true);
-            tryFinalize(it);
+            scheduleFinalizationAtSafepoint(it);
           }
           if (killed) {
             return;
@@ -229,18 +246,16 @@ public Item(Object underlying, Object finalizer, PhantomReference<ManagedResourc
     }
 
     /**
-     * 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);
       }
     }
 

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")

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
       }