[GR-18163] Thread#raise should call #exception in the target thread

eregon · eregon · commit c859b6af595f · 2022-05-13T15:46:20.000Z
PullRequest: truffleruby/3349
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -27,6 +27,7 @@ Compatibility:
 * Fix `Float#/` when dividing by `Rational` (@bjfish).
 * `Process.euid=` should accept String (#2615, @ngtban).
 * Fix `instance_variable_get` and `instance_variable_set` for immutable objects (@bjfish).
+* `Thread#raise(exc, message)` now calls `exc.exception` in the target thread like CRuby (@eregon).
 
 Performance:
 
diff --git a/spec/ruby/core/thread/raise_spec.rb b/spec/ruby/core/thread/raise_spec.rb
@@ -102,6 +102,30 @@
       raised_again.backtrace.first.should_not include("#{__FILE__}:#{raise_again_line}:")
     end
   end
+
+  it "calls #exception in both the caller and in the target thread" do
+    cls = Class.new(Exception) do
+      attr_accessor :log
+      def initialize(*args)
+        @log = [] # This is shared because the super #exception uses a shallow clone
+        super
+      end
+
+      def exception(*args)
+        @log << [self, Thread.current, args]
+        super
+      end
+    end
+    exc = cls.new
+
+    @thr.raise exc, "Thread#raise #exception spec"
+    @thr.join
+    ScratchPad.recorded.should.is_a?(cls)
+    exc.log.should == [
+      [exc, Thread.current, ["Thread#raise #exception spec"]],
+      [ScratchPad.recorded, @thr, []]
+    ]
+  end
 end
 
 describe "Thread#raise on a running thread" do
diff --git a/src/main/java/org/truffleruby/RubyContext.java b/src/main/java/org/truffleruby/RubyContext.java
@@ -436,23 +436,18 @@ private TruffleNFIPlatform createNativePlatform() {
         return truffleNFIPlatform;
     }
 
-    @TruffleBoundary
-    public static Object send(Object receiver, String methodName, Object... arguments) {
-        return DispatchNode.getUncached().call(receiver, methodName, arguments);
-    }
-
     @TruffleBoundary
     public static Object send(Node currentNode, Object receiver, String methodName, Object... arguments) {
         if (currentNode.isAdoptable()) {
             final EncapsulatingNodeReference callNodeRef = EncapsulatingNodeReference.getCurrent();
             final Node prev = callNodeRef.set(currentNode);
             try {
-                return send(receiver, methodName, arguments);
+                return DispatchNode.getUncached().call(receiver, methodName, arguments);
             } finally {
                 callNodeRef.set(prev);
             }
         } else {
-            return send(receiver, methodName, arguments);
+            return DispatchNode.getUncached().call(receiver, methodName, arguments);
         }
     }
 
diff --git a/src/main/java/org/truffleruby/core/adapters/InputStreamAdapter.java b/src/main/java/org/truffleruby/core/adapters/InputStreamAdapter.java
@@ -10,9 +10,9 @@
 package org.truffleruby.core.adapters;
 
 import org.graalvm.shadowed.org.jline.utils.NonBlockingInputStream;
-import org.truffleruby.RubyContext;
 import org.truffleruby.core.support.RubyIO;
 import org.truffleruby.language.Nil;
+import org.truffleruby.language.dispatch.DispatchNode;
 
 /** extends NonBlockingInputStream so JLine does not create an extra thread and calls read() on the same thread as
  * calling readLine(). */
@@ -28,7 +28,7 @@ public InputStreamAdapter(RubyIO object) {
 
     @Override
     public int read() {
-        final Object result = RubyContext.send(object, "getbyte");
+        final Object result = DispatchNode.getUncached().call(object, "getbyte");
 
         if (result == Nil.INSTANCE) {
             return EOF;
diff --git a/src/main/java/org/truffleruby/core/adapters/OutputStreamAdapter.java b/src/main/java/org/truffleruby/core/adapters/OutputStreamAdapter.java
@@ -18,6 +18,7 @@
 import org.truffleruby.core.rope.RopeOperations;
 import org.truffleruby.core.string.StringOperations;
 import org.truffleruby.core.support.RubyIO;
+import org.truffleruby.language.dispatch.DispatchNode;
 
 public class OutputStreamAdapter extends OutputStream {
 
@@ -35,15 +36,8 @@ public OutputStreamAdapter(RubyContext context, RubyLanguage language, RubyIO ob
 
     @Override
     public void write(int bite) {
-        RubyContext.send(
-                object,
-                "write",
-                StringOperations
-                        .createString(
-                                context,
-                                language,
-                                RopeOperations.create((byte) bite, encoding.jcoding, CodeRange.CR_UNKNOWN),
-                                encoding));
+        DispatchNode.getUncached().call(object, "write", StringOperations.createString(context, language,
+                RopeOperations.create((byte) bite, encoding.jcoding, CodeRange.CR_UNKNOWN), encoding));
     }
 
 }
diff --git a/src/main/java/org/truffleruby/core/exception/CoreExceptions.java b/src/main/java/org/truffleruby/core/exception/CoreExceptions.java
@@ -51,6 +51,7 @@
 import com.oracle.truffle.api.nodes.Node;
 import com.oracle.truffle.api.object.DynamicObjectLibrary;
 import com.oracle.truffle.api.source.SourceSection;
+import org.truffleruby.language.dispatch.DispatchNode;
 import org.truffleruby.language.library.RubyStringLibrary;
 import org.truffleruby.language.objects.LogicalClassNode;
 
@@ -80,7 +81,7 @@ public void showExceptionIfDebug(RubyException exception) {
     public void showExceptionIfDebug(RubyException rubyException, Backtrace backtrace) {
         if (context.getCoreLibrary().getDebug() == Boolean.TRUE) {
             final RubyClass rubyClass = rubyException.getLogicalClass();
-            final Object message = RubyContext.send(rubyException, "to_s");
+            final Object message = DispatchNode.getUncached().call(rubyException, "to_s");
             showExceptionIfDebug(rubyClass, message, backtrace);
         }
     }
@@ -104,7 +105,7 @@ public void showExceptionIfDebug(RubyClass rubyClass, Object message, Backtrace
                                 language,
                                 StringOperations.encodeRope(output, UTF8Encoding.INSTANCE));
                 Object stderr = context.getCoreLibrary().getStderr();
-                RubyContext.send(stderr, "write", outputString);
+                DispatchNode.getUncached().call(stderr, "write", outputString);
             } else {
                 context.getEnvErrStream().println(output);
             }
@@ -113,16 +114,15 @@ public void showExceptionIfDebug(RubyClass rubyClass, Object message, Backtrace
 
     @TruffleBoundary
     public String inspect(Object value) {
-        Object rubyString = RubyContext.send(context.getCoreLibrary().truffleTypeModule, "rb_inspect", value);
+        Object rubyString = DispatchNode.getUncached().call(
+                context.getCoreLibrary().truffleTypeModule, "rb_inspect", value);
         return RubyStringLibrary.getUncached().getJavaString(rubyString);
     }
 
     @TruffleBoundary
     public String inspectReceiver(Object receiver) {
-        Object rubyString = RubyContext.send(
-                context.getCoreLibrary().truffleExceptionOperationsModule,
-                "receiver_string",
-                receiver);
+        Object rubyString = DispatchNode.getUncached().call(
+                context.getCoreLibrary().truffleExceptionOperationsModule, "receiver_string", receiver);
         return RubyStringLibrary.getUncached().getJavaString(rubyString);
     }
 
@@ -686,7 +686,8 @@ public RubyException typeError(String message, Node currentNode, Throwable javaT
     @TruffleBoundary
     public RubyException typeErrorUnsupportedTypeException(UnsupportedTypeException exception, Node currentNode) {
         RubyArray rubyArray = createArray(context, language, exception.getSuppliedValues());
-        String formattedValues = RubyStringLibrary.getUncached().getJavaString(RubyContext.send(rubyArray, "inspect"));
+        String formattedValues = RubyStringLibrary.getUncached()
+                .getJavaString(DispatchNode.getUncached().call(rubyArray, "inspect"));
         return typeError("unsupported type " + formattedValues, currentNode);
     }
 
diff --git a/src/main/java/org/truffleruby/core/exception/ExceptionOperations.java b/src/main/java/org/truffleruby/core/exception/ExceptionOperations.java
@@ -28,6 +28,7 @@
 import com.oracle.truffle.api.nodes.Node;
 import com.oracle.truffle.api.object.Shape;
 import org.truffleruby.language.control.RaiseException;
+import org.truffleruby.language.dispatch.DispatchNode;
 import org.truffleruby.language.library.RubyStringLibrary;
 
 public abstract class ExceptionOperations {
@@ -112,7 +113,7 @@ private static String messageFieldToString(RubyException exception) {
     public static String messageToString(RubyException exception) {
         Object messageObject = null;
         try {
-            messageObject = RubyContext.send(exception, "message");
+            messageObject = DispatchNode.getUncached().call(exception, "message");
         } catch (Throwable e) {
             // Fall back to the internal message field
         }
diff --git a/src/main/java/org/truffleruby/core/hash/PreInitializationManager.java b/src/main/java/org/truffleruby/core/hash/PreInitializationManager.java
@@ -15,9 +15,8 @@
 import java.util.Set;
 import java.util.WeakHashMap;
 
-import org.truffleruby.RubyContext;
-
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
+import org.truffleruby.language.dispatch.DispatchNode;
 
 public final class PreInitializationManager {
 
@@ -53,7 +52,7 @@ public void rehash() {
     private void rehashRubyHashes() {
         for (RubyHash hash : hashesCreatedDuringPreInit) {
             if (!HashGuards.isCompareByIdentity(hash)) {
-                RubyContext.send(hash, "rehash");
+                DispatchNode.getUncached().call(hash, "rehash");
             }
         }
         hashesCreatedDuringPreInit.clear();
diff --git a/src/main/java/org/truffleruby/core/objectspace/ObjectSpaceNodes.java b/src/main/java/org/truffleruby/core/objectspace/ObjectSpaceNodes.java
@@ -11,7 +11,6 @@
 
 import com.oracle.truffle.api.object.DynamicObjectLibrary;
 import org.truffleruby.Layouts;
-import org.truffleruby.RubyContext;
 import org.truffleruby.builtins.CoreMethod;
 import org.truffleruby.builtins.CoreMethodArrayArgumentsNode;
 import org.truffleruby.builtins.CoreModule;
@@ -34,6 +33,7 @@
 import org.truffleruby.language.RubyDynamicObject;
 import org.truffleruby.language.RubyGuards;
 import org.truffleruby.language.control.RaiseException;
+import org.truffleruby.language.dispatch.DispatchNode;
 import org.truffleruby.language.dispatch.InternalRespondToNode;
 import org.truffleruby.language.objects.IsANode;
 import org.truffleruby.language.objects.ObjectGraph;
@@ -185,7 +185,7 @@ public CallableFinalizer(Object callable) {
         }
 
         public void run() {
-            RubyContext.send(callable, "call");
+            DispatchNode.getUncached().call(callable, "call");
         }
 
         @Override
diff --git a/src/main/java/org/truffleruby/core/thread/ThreadManager.java b/src/main/java/org/truffleruby/core/thread/ThreadManager.java
@@ -56,6 +56,7 @@
 import org.truffleruby.language.control.ExitException;
 import org.truffleruby.language.control.KillException;
 import org.truffleruby.language.control.RaiseException;
+import org.truffleruby.language.dispatch.DispatchNode;
 import org.truffleruby.language.objects.shared.SharedObjects;
 
 import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
@@ -375,14 +376,10 @@ private void setException(RubyThread thread, RubyException exception, Node curre
 
             if (!isSystemExit && thread.reportOnException) {
                 final TruffleSafepoint safepoint = TruffleSafepoint.getCurrent();
-
                 boolean sideEffects = safepoint.setAllowSideEffects(false);
                 try {
-                    RubyContext.send(
-                            context.getCoreLibrary().truffleThreadOperationsModule,
-                            "report_exception",
-                            thread,
-                            exception);
+                    DispatchNode.getUncached().call(context.getCoreLibrary().truffleThreadOperationsModule,
+                            "report_exception", thread, exception);
                 } finally {
                     safepoint.setAllowSideEffects(sideEffects);
                 }
diff --git a/src/main/java/org/truffleruby/core/thread/ThreadNodes.java b/src/main/java/org/truffleruby/core/thread/ThreadNodes.java
@@ -858,11 +858,14 @@ public static void raiseInThread(RubyLanguage language, RubyContext context, Rub
                     new SafepointAction("Thread#raise", rubyThread, true, false) {
                         @Override
                         public void run(RubyThread rubyThread, Node currentNode) {
-                            if (exception.backtrace == null) {
-                                exception.backtrace = context.getCallStack().getBacktrace(currentNode);
+                            final TruffleSafepoint safepoint = TruffleSafepoint.getCurrent();
+                            boolean sideEffects = safepoint.setAllowSideEffects(false);
+                            try {
+                                RubyContext.send(currentNode, rubyThread, "raise", exception);
+                            } finally {
+                                safepoint.setAllowSideEffects(sideEffects);
                             }
-
-                            VMRaiseExceptionNode.reRaiseException(context, exception);
+                            throw CompilerDirectives.shouldNotReachHere("#raise did not throw?");
                         }
                     });
         }
diff --git a/src/main/java/org/truffleruby/language/EmitWarningsNode.java b/src/main/java/org/truffleruby/language/EmitWarningsNode.java
@@ -17,6 +17,7 @@
 import org.truffleruby.core.string.RubyString;
 import org.truffleruby.core.string.StringOperations;
 import org.truffleruby.language.control.RaiseException;
+import org.truffleruby.language.dispatch.DispatchNode;
 import org.truffleruby.parser.RubyDeferredWarnings;
 
 import java.io.IOException;
@@ -70,7 +71,7 @@ private static void printWarning(RubyContext context, String message) {
             final Rope messageRope = StringOperations.encodeRope(message, UTF8Encoding.INSTANCE);
             final RubyString messageString = StringOperations
                     .createUTF8String(context, context.getLanguageSlow(), messageRope);
-            RubyContext.send(warning, "warn", messageString);
+            DispatchNode.getUncached().call(warning, "warn", messageString);
         } else {
             try {
                 context.getEnvErrStream().write(message.getBytes(StandardCharsets.UTF_8));
diff --git a/src/main/java/org/truffleruby/language/RubyEvalInteractiveRootNode.java b/src/main/java/org/truffleruby/language/RubyEvalInteractiveRootNode.java
@@ -20,6 +20,7 @@
 
 import com.oracle.truffle.api.frame.VirtualFrame;
 import com.oracle.truffle.api.source.Source;
+import org.truffleruby.language.dispatch.DispatchNode;
 
 public class RubyEvalInteractiveRootNode extends RubyBaseRootNode implements InternalRootNode {
 
@@ -40,11 +41,8 @@ public Object execute(VirtualFrame frame) {
         // Just do Truffle::Boot::INTERACTIVE_BINDING.eval(code) for interactive sources.
         // It's the semantics we want and takes care of caching correctly based on the Binding's FrameDescriptor.
         final RubyBinding interactiveBinding = context.getCoreLibrary().interactiveBinding;
-        return RubyContext
-                .send(
-                        interactiveBinding,
-                        "eval",
-                        StringOperations.createString(this, sourceRope, Encodings.UTF_8));
+        return DispatchNode.getUncached().call(interactiveBinding, "eval",
+                StringOperations.createString(this, sourceRope, Encodings.UTF_8));
     }
 
     @Override
diff --git a/src/main/java/org/truffleruby/language/backtrace/BacktraceFormatter.java b/src/main/java/org/truffleruby/language/backtrace/BacktraceFormatter.java
@@ -29,6 +29,7 @@
 import org.truffleruby.core.string.StringUtils;
 import org.truffleruby.language.RubyRootNode;
 import org.truffleruby.language.control.RaiseException;
+import org.truffleruby.language.dispatch.DispatchNode;
 import org.truffleruby.language.library.RubyStringLibrary;
 import org.truffleruby.language.methods.TranslateExceptionNode;
 import org.truffleruby.parser.RubySource;
@@ -132,7 +133,7 @@ public void printRubyExceptionOnEnvStderr(String info, AbstractTruffleException
         }
 
         try {
-            final Object fullMessage = RubyContext.send(
+            final Object fullMessage = DispatchNode.getUncached().call(
                     context.getCoreLibrary().truffleExceptionOperationsModule,
                     "get_formatted_backtrace",
                     exceptionObject);
diff --git a/src/main/java/org/truffleruby/language/loader/FeatureLoader.java b/src/main/java/org/truffleruby/language/loader/FeatureLoader.java
@@ -206,7 +206,7 @@ private String initializeWorkingDirectory() {
                 throw CompilerDirectives.shouldNotReachHere(e);
             }
             if (address == 0) {
-                RubyContext.send(context.getCoreLibrary().errnoModule, "handle");
+                DispatchNode.getUncached().call(context.getCoreLibrary().errnoModule, "handle");
             }
             final byte[] bytes = buffer.readZeroTerminatedByteArray(
                     context,
@@ -440,7 +440,7 @@ public void ensureCExtImplementationLoaded(String feature, RequireNode requireNo
                                 context,
                                 language,
                                 StringOperations.encodeRope("truffle/cext", UTF8Encoding.INSTANCE));
-                RubyContext.send(context.getCoreLibrary().mainObject, "gem_original_require", cextRb);
+                DispatchNode.getUncached().call(context.getCoreLibrary().mainObject, "gem_original_require", cextRb);
 
                 final RubyModule truffleModule = context.getCoreLibrary().truffleModule;
                 final Object truffleCExt = truffleModule.fields.getConstant("CExt").getValue();
@@ -505,7 +505,8 @@ public Object loadCExtLibrary(String feature, String path, Node currentNode) {
             final Object library = context.getEnv().parseInternal(source).call();
 
             final Object embeddedABIVersion = getEmbeddedABIVersion(path, library);
-            RubyContext.send(context.getCoreLibrary().truffleCExtModule, "check_abi_version", embeddedABIVersion, path);
+            DispatchNode.getUncached().call(context.getCoreLibrary().truffleCExtModule, "check_abi_version",
+                    embeddedABIVersion, path);
 
             return library;
         } finally {
diff --git a/src/main/java/org/truffleruby/language/objects/ForeignClassNode.java b/src/main/java/org/truffleruby/language/objects/ForeignClassNode.java
@@ -12,7 +12,6 @@
 import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.dsl.Cached;
 import com.oracle.truffle.api.dsl.ReportPolymorphism;
-import org.truffleruby.RubyContext;
 import org.truffleruby.core.klass.RubyClass;
 import org.truffleruby.core.symbol.RubySymbol;
 import org.truffleruby.language.RubyBaseNode;
@@ -22,6 +21,7 @@
 import com.oracle.truffle.api.interop.InteropLibrary;
 import com.oracle.truffle.api.library.CachedLibrary;
 import org.truffleruby.language.RubyGuards;
+import org.truffleruby.language.dispatch.DispatchNode;
 
 import java.util.ArrayList;
 
@@ -116,7 +116,7 @@ private RubyClass resolvePolyglotForeignClass(int traits) {
             }
         }
         final Object[] traitSymbols = traitsList.toArray();
-        return (RubyClass) RubyContext
-                .send(coreLibrary().truffleInteropOperationsModule, "resolve_polyglot_class", traitSymbols);
+        return (RubyClass) DispatchNode.getUncached().call(coreLibrary().truffleInteropOperationsModule,
+                "resolve_polyglot_class", traitSymbols);
     }
 }
diff --git a/src/main/java/org/truffleruby/parser/RubyWarnings.java b/src/main/java/org/truffleruby/parser/RubyWarnings.java
diff --git a/src/main/java/org/truffleruby/stdlib/readline/ReadlineNodes.java b/src/main/java/org/truffleruby/stdlib/readline/ReadlineNodes.java

Original file line number	Diff line number	Diff line change
`@@ -436,23 +436,18 @@ private TruffleNFIPlatform createNativePlatform() {`
`436`	`436`	`return truffleNFIPlatform;`
`437`	`437`	`}`
`438`	`438`
`439`		`- @TruffleBoundary`
`440`		`- public static Object send(Object receiver, String methodName, Object... arguments) {`
`441`		`- return DispatchNode.getUncached().call(receiver, methodName, arguments);`
`442`		`- }`
`443`		`-`
`444`	`439`	`@TruffleBoundary`
`445`	`440`	`public static Object send(Node currentNode, Object receiver, String methodName, Object... arguments) {`
`446`	`441`	`if (currentNode.isAdoptable()) {`
`447`	`442`	`final EncapsulatingNodeReference callNodeRef = EncapsulatingNodeReference.getCurrent();`
`448`	`443`	`final Node prev = callNodeRef.set(currentNode);`
`449`	`444`	`try {`
`450`		`- return send(receiver, methodName, arguments);`
	`445`	`+ return DispatchNode.getUncached().call(receiver, methodName, arguments);`
`451`	`446`	`} finally {`
`452`	`447`	`callNodeRef.set(prev);`
`453`	`448`	`}`
`454`	`449`	`} else {`
`455`		`- return send(receiver, methodName, arguments);`
	`450`	`+ return DispatchNode.getUncached().call(receiver, methodName, arguments);`
`456`	`451`	`}`
`457`	`452`	`}`
`458`	`453`