[GR-51302] RData/RTypedData finalizers should use a wrapper

* So if a Java exception happens in native code it is handled correctly.
oracle · Jan 12, 2024 · d06b0e3 · d06b0e3
1 parent 2e95271
commit d06b0e3
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 42 deletions.
diff --git a/lib/truffle/truffle/cext.rb b/lib/truffle/truffle/cext.rb
@@ -1728,6 +1728,10 @@ def rb_data_typed_object_wrap(ruby_class, data, data_type, mark, free, size)
     object
   end
 
+  def run_data_finalizer(function, data)
+    Primitive.call_with_c_mutex_and_frame POINTER_TO_VOID_WRAPPER, [function, data], nil, nil
+  end
+
   def run_marker(obj)
     Primitive.array_mark_store(obj) if Primitive.array_store_native?(obj)
 

diff --git a/src/main/java/org/truffleruby/core/DataObjectFinalizationService.java b/src/main/java/org/truffleruby/core/DataObjectFinalizationService.java
@@ -10,24 +10,18 @@
 package org.truffleruby.core;
 
 import java.lang.ref.ReferenceQueue;
-import java.util.concurrent.locks.ReentrantLock;
 
+import com.oracle.truffle.api.interop.InteropException;
 import org.truffleruby.RubyContext;
 import org.truffleruby.RubyLanguage;
-import org.truffleruby.core.MarkingService.ExtensionCallStack;
-import org.truffleruby.core.mutex.MutexOperations;
 import org.truffleruby.language.Nil;
 import org.truffleruby.language.RubyBaseRootNode;
 import org.truffleruby.language.backtrace.InternalRootNode;
 
 import com.oracle.truffle.api.CallTarget;
 import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
-import com.oracle.truffle.api.interop.ArityException;
 import com.oracle.truffle.api.interop.InteropLibrary;
-import com.oracle.truffle.api.interop.UnsupportedMessageException;
-import com.oracle.truffle.api.interop.UnsupportedTypeException;
-import com.oracle.truffle.api.profiles.ConditionProfile;
 import com.oracle.truffle.api.frame.VirtualFrame;
 
 /** C-ext data finalizers are implemented with phantom references and reference queues, and are run in a dedicated Ruby
@@ -40,10 +34,8 @@ public final class DataObjectFinalizationService
     public static final class DataObjectFinalizerRootNode extends RubyBaseRootNode implements InternalRootNode {
 
         @Child private InteropLibrary callNode;
-        private final ConditionProfile ownedProfile = ConditionProfile.create();
 
-        public DataObjectFinalizerRootNode(
-                RubyLanguage language) {
+        public DataObjectFinalizerRootNode(RubyLanguage language) {
             super(language, RubyLanguage.EMPTY_FRAME_DESCRIPTOR, null);
 
             callNode = InteropLibrary.getFactory().createDispatched(1);
@@ -55,41 +47,17 @@ public Object execute(VirtualFrame frame) {
         }
 
         public Object execute(DataObjectFinalizerReference ref) {
-            if (getContext().getOptions().CEXT_LOCK) {
-                final ReentrantLock lock = getContext().getCExtensionsLock();
-                boolean owned = ownedProfile.profile(lock.isHeldByCurrentThread());
-
-                if (!owned) {
-                    MutexOperations.lockInternal(getContext(), lock, this);
-                }
+            if (!getContext().isFinalizing()) {
                 try {
-                    runFinalizer(ref);
-                } finally {
-                    if (!owned) {
-                        MutexOperations.unlockInternal(lock);
-                    }
+                    callNode.invokeMember(getContext().getCoreLibrary().truffleCExtModule, "run_data_finalizer",
+                            ref.finalizerCFunction, ref.dataStruct);
+                } catch (InteropException e) {
+                    throw CompilerDirectives.shouldNotReachHere(e);
                 }
-            } else {
-                runFinalizer(ref);
             }
             return Nil.INSTANCE;
         }
 
-        private void runFinalizer(DataObjectFinalizerReference ref) throws Error {
-            try {
-                if (!getContext().isFinalizing()) {
-                    final ExtensionCallStack stack = getLanguage().getCurrentFiber().extensionCallStack;
-                    stack.push(false, stack.getSpecialVariables(), stack.getBlock());
-                    try {
-                        callNode.execute(ref.finalizerCFunction, ref.dataStruct);
-                    } finally {
-                        stack.pop();
-                    }
-                }
-            } catch (UnsupportedTypeException | ArityException | UnsupportedMessageException e) {
-                throw CompilerDirectives.shouldNotReachHere("Data holder finalization on invalid object");
-            }
-        }
     }
 
     private final CallTarget callTarget;
@@ -127,8 +95,7 @@ protected void processReference(RubyContext context, RubyLanguage language,
     }
 
     @TruffleBoundary
-    protected void processReferenceInternal(RubyContext context, RubyLanguage language,
-            DataObjectFinalizerReference ref) {
-        callTarget.call(new Object[]{ ref });
+    void processReferenceInternal(RubyContext context, RubyLanguage language, DataObjectFinalizerReference ref) {
+        callTarget.call(ref);
     }
 }