[GR-45825] Support cancellation with Fibers

PullRequest: truffleruby/3805

Author: eregon

ci.jsonnet

@@ -211,7 +211,7 @@ local part_definitions = {
     host_inlining_log: {
       # Same as in mx.truffleruby/native-host-inlining
       mx_options+:: [
-        "--extra-image-builder-argument=rubyvm:-H:Log=TruffleHostInliningPhase,~CanonicalizerPhase,~GraphBuilderPhase",
+        "--extra-image-builder-argument=rubyvm:-H:Log=HostInliningPhase,~CanonicalizerPhase,~GraphBuilderPhase",
         "--extra-image-builder-argument=rubyvm:-H:+TruffleHostInliningPrintExplored",
         "--extra-image-builder-argument=rubyvm:-Dgraal.LogFile=host-inlining.txt",
       ],

common.json

@@ -4,7 +4,7 @@
     "Jsonnet files should not include this file directly but use ci/common.jsonnet instead."
   ],
 
-  "mx_version": "6.19.3",
+  "mx_version": "6.20.3",
 
   "COMMENT.jdks": "When adding or removing JDKs keep in sync with JDKs in ci/common.jsonnet",
   "jdks": {

mx.truffleruby/native-host-inlining

@@ -2,6 +2,6 @@ GRAALVM_SKIP_ARCHIVE=true
 DYNAMIC_IMPORTS=/tools,/compiler,/substratevm
 COMPONENTS=TruffleRuby,suite:tools,GraalVM compiler,SubstrateVM,Native Image
 NATIVE_IMAGES=lib:rubyvm
-EXTRA_IMAGE_BUILDER_ARGUMENTS=rubyvm:-H:Log=TruffleHostInliningPhase,~CanonicalizerPhase,~GraphBuilderPhase rubyvm:-H:+TruffleHostInliningPrintExplored rubyvm:-Dgraal.LogFile=host-inlining.txt
+EXTRA_IMAGE_BUILDER_ARGUMENTS=rubyvm:-H:Log=HostInliningPhase,~CanonicalizerPhase,~GraphBuilderPhase rubyvm:-H:+TruffleHostInliningPrintExplored rubyvm:-Dgraal.LogFile=host-inlining.txt
 # To also create the standalone
 DISABLE_INSTALLABLES=false

mx.truffleruby/suite.py

@@ -7,7 +7,7 @@
             {
                 "name": "regex",
                 "subdir": True,
-                "version": "db464f45d94a76851b5d1c9e6c2401157c9b9ac5",
+                "version": "691c375e722825f333ce933e7164fbd7fde8fc44",
                 "urls": [
                     {"url": "https://github.com/oracle/graal.git", "kind": "git"},
                     {"url": "https://curio.ssw.jku.at/nexus/content/repositories/snapshots", "kind": "binary"},
@@ -16,7 +16,7 @@
             {
                 "name": "sulong",
                 "subdir": True,
-                "version": "db464f45d94a76851b5d1c9e6c2401157c9b9ac5",
+                "version": "691c375e722825f333ce933e7164fbd7fde8fc44",
                 "urls": [
                     {"url": "https://github.com/oracle/graal.git", "kind": "git"},
                     {"url": "https://curio.ssw.jku.at/nexus/content/repositories/snapshots", "kind": "binary"},

src/main/java/org/truffleruby/core/fiber/FiberManager.java

@@ -15,7 +15,6 @@
 import com.oracle.truffle.api.TruffleSafepoint.Interrupter;
 import org.truffleruby.core.fiber.RubyFiber.FiberStatus;
 
-import com.oracle.truffle.api.TruffleContext;
 import com.oracle.truffle.api.TruffleSafepoint;
 import org.truffleruby.RubyContext;
 import org.truffleruby.RubyLanguage;
@@ -75,9 +74,8 @@ private void createThreadToReceiveFirstMessage(RubyFiber fiber, Node currentNode
         fiber.initializeNode = null;
 
         var sourceSection = block.getSharedMethodInfo().getSourceSection();
-        final TruffleContext truffleContext = context.getEnv().getContext();
 
-        truffleContext.leaveAndEnter(currentNode, Interrupter.THREAD_INTERRUPT, (unused) -> {
+        context.getThreadManager().leaveAndEnter(currentNode, Interrupter.THREAD_INTERRUPT, (unused) -> {
             Thread thread = context.getThreadManager().createFiberJavaThread(fiber, sourceSection,
                     () -> beforeEnter(fiber, initializeNode),
                     () -> fiberMain(context, fiber, block, initializeNode),
@@ -158,6 +156,9 @@ private void fiberMain(RubyContext context, RubyFiber fiber, RubyProc block, Nod
         } catch (FiberShutdownException e) {
             // Ends execution of the Fiber
             lastMessage = null;
+        } catch (ThreadDeath e) { // Context#close(true), handled by Truffle
+            lastMessage = null;
+            throw e;
         } catch (BreakException e) {
             final RubyException exception = context.getCoreExceptions().breakFromProcClosure(currentNode);
             lastMessage = new FiberExceptionMessage(new RaiseException(context, exception));
@@ -223,7 +224,7 @@ private void addToMessageQueue(RubyFiber fiber, FiberMessage message) {
     @TruffleBoundary
     private FiberMessage waitMessage(RubyFiber fiber, Node currentNode) throws InterruptedException {
         assertNotEntered("should have left context while waiting fiber message");
-        return fiber.messageQueue.take();
+        return Objects.requireNonNull(fiber.messageQueue.take());
     }
 
     private void assertNotEntered(String reason) {
@@ -314,8 +315,7 @@ private FiberMessage resumeAndWait(RubyFiber fromFiber, RubyFiber toFiber, Fiber
             context.fiberManager.createThreadToReceiveFirstMessage(toFiber, currentNode);
         }
 
-        final TruffleContext truffleContext = context.getEnv().getContext();
-        final FiberMessage message = truffleContext.leaveAndEnter(currentNode, Interrupter.THREAD_INTERRUPT,
+        var message = context.getThreadManager().leaveAndEnter(currentNode, Interrupter.THREAD_INTERRUPT,
                 (unused) -> {
                     resume(fromFiber, toFiber, operation, descriptor, args);
                     return waitMessage(fromFiber, currentNode);
@@ -326,8 +326,7 @@ private FiberMessage resumeAndWait(RubyFiber fromFiber, RubyFiber toFiber, Fiber
 
     @TruffleBoundary
     public void safepoint(RubyFiber fromFiber, RubyFiber fiber, SafepointAction action, Node currentNode) {
-        final TruffleContext truffleContext = context.getEnv().getContext();
-        final FiberResumeMessage returnMessage = (FiberResumeMessage) truffleContext.leaveAndEnter(currentNode,
+        var returnMessage = (FiberResumeMessage) context.getThreadManager().leaveAndEnter(currentNode,
                 Interrupter.THREAD_INTERRUPT, (unused) -> {
                     addToMessageQueue(fiber, new FiberSafepointMessage(fromFiber, action));
                     return waitMessage(fromFiber, currentNode);
@@ -382,8 +381,7 @@ public void killOtherFibers(RubyThread thread) {
         final TruffleSafepoint safepoint = TruffleSafepoint.getCurrent();
         boolean allowSideEffects = safepoint.setAllowSideEffects(false);
         try {
-            final TruffleContext truffleContext = context.getEnv().getContext();
-            truffleContext.leaveAndEnter(DummyNode.INSTANCE, Interrupter.THREAD_INTERRUPT, (unused) -> {
+            context.getThreadManager().leaveAndEnter(DummyNode.INSTANCE, Interrupter.THREAD_INTERRUPT, (unused) -> {
                 doKillOtherFibers(thread);
                 return BlockingAction.SUCCESS;
             }, null);

src/main/java/org/truffleruby/core/thread/ThreadManager.java

@@ -11,6 +11,7 @@
 
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.util.Objects;
 import java.util.Set;
 import java.util.Timer;
 import java.util.concurrent.ConcurrentHashMap;
@@ -23,6 +24,7 @@
 import com.oracle.truffle.api.CompilerAsserts;
 import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.CompilerDirectives.ValueType;
+import com.oracle.truffle.api.TruffleContext;
 import com.oracle.truffle.api.TruffleOptions;
 import com.oracle.truffle.api.TruffleSafepoint;
 import com.oracle.truffle.api.TruffleSafepoint.Interrupter;
@@ -607,6 +609,15 @@ public <T> T runUntilResult(Node currentNode, BlockingAction<T> action, Runnable
         }
     }
 
+    /** Same as {@link TruffleContext#leaveAndEnter} but ensures it never returns null. Also the interruptible must
+     * never return null. */
+    public <T, R> R leaveAndEnter(Node node, Interrupter interrupter, InterruptibleFunction<T, R> interruptible,
+            T object) {
+        final TruffleContext truffleContext = context.getEnv().getContext();
+        R result = truffleContext.leaveAndEnter(node, interrupter, interruptible, object);
+        return Objects.requireNonNull(result);
+    }
+
     @TruffleBoundary
     Interrupter getNativeCallInterrupter() {
         if (nativeInterrupt) {

src/test/java/org/truffleruby/MiscTest.java

@@ -15,9 +15,7 @@
 import static org.junit.Assert.assertTrue;
 
 import org.graalvm.polyglot.Context;
-import org.graalvm.polyglot.PolyglotException;
 import org.graalvm.polyglot.Value;
-import org.junit.Assert;
 import org.junit.Test;
 
 public class MiscTest {
@@ -49,26 +47,43 @@ public void timeoutExecution() throws Throwable {
         TestingThread thread = new TestingThread(() -> {
             try {
                 Thread.sleep(1000);
-                context.close(true);
             } catch (InterruptedException e) {
                 throw new Error(e);
-            } catch (PolyglotException e) {
-                if (e.isCancelled()) {
-                    assertTrue(e.isCancelled());
-                } else {
-                    throw e;
-                }
             }
+            context.close(true);
         });
 
         context.eval("ruby", "init = 1");
         thread.start();
         try {
             String maliciousCode = "while true; end";
-            context.eval("ruby", maliciousCode);
-            Assert.fail();
-        } catch (PolyglotException e) {
-            assertTrue(e.isCancelled());
+            RubyTest.assertThrows(() -> context.eval("ruby", maliciousCode),
+                    e -> assertTrue(e.isCancelled()));
+        } finally {
+            thread.join();
+        }
+    }
+
+    @Test
+    public void testCancellationWithFibers() throws Throwable {
+        Context context = RubyTest.createContext();
+
+        // schedule a timeout in 100ms
+        TestingThread thread = new TestingThread(() -> {
+            try {
+                Thread.sleep(100);
+            } catch (InterruptedException e) {
+                throw new Error(e);
+            }
+            context.close(true);
+        });
+
+        context.eval("ruby", "init = 1");
+        thread.start();
+        try {
+            String code = "unstarted = Fiber.new {}; resumed = Fiber.new { Fiber.yield }.tap(&:resume); sleep 1";
+            RubyTest.assertThrows(() -> context.eval("ruby", code),
+                    e -> assertTrue(e.isCancelled()));
         } finally {
             thread.join();
         }