Skip to content

Commit

Permalink
[GR-45825] Support cancellation with Fibers
Browse files Browse the repository at this point in the history
PullRequest: truffleruby/3805
  • Loading branch information
eregon committed May 8, 2023
2 parents 3a4fe79 + 3560b0f commit 6f0ce72
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 28 deletions.
2 changes: 1 addition & 1 deletion ci.jsonnet
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
Expand Down
2 changes: 1 addition & 1 deletion common.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
2 changes: 1 addition & 1 deletion mx.truffleruby/native-host-inlining
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions mx.truffleruby/suite.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand All @@ -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"},
Expand Down
18 changes: 8 additions & 10 deletions src/main/java/org/truffleruby/core/fiber/FiberManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
11 changes: 11 additions & 0 deletions src/main/java/org/truffleruby/core/thread/ThreadManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
41 changes: 28 additions & 13 deletions src/test/java/org/truffleruby/MiscTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
}
Expand Down

0 comments on commit 6f0ce72

Please sign in to comment.