[GR-17457] Unwrap values returned from C before popping the extension…

… stack.

PullRequest: truffleruby/3246

CHANGELOG.md

@@ -20,6 +20,7 @@ Bug fixes:
 * Fix `Random#rand` not returning random floats when given float ranges (#2612, @bjfish).
 * Fix `Array#sample` for `[]` when called without `n` and a `Random` is given (#2612, @bjfish).
 * Fix `Module#const_get` to raise a `NameError` when nested modules do not exist (#2610, @bjfish).
+* Ensure native `VALUE`s returned from C are unwrapped before the objects can be collected (@aardvark179).
 
 Compatibility:
 

lib/truffle/truffle/cext.rb

@@ -975,14 +975,13 @@ def rb_path_to_class(path)
 
   def rb_proc_new(function, value)
     Proc.new do |*args, &block|
-      Primitive.cext_unwrap(
-          Primitive.call_with_c_mutex_and_frame(function, [
-              Primitive.cext_wrap(args.first), # yieldarg
-              Primitive.cext_wrap(value), # procarg,
-              args.size, # argc
-              Truffle::CExt.RARRAY_PTR(args), # argv
-              Primitive.cext_wrap(block), # blockarg
-          ], Primitive.caller_special_variables_if_available, nil))
+      Primitive.call_with_c_mutex_and_frame_and_unwrap(function, [
+        Primitive.cext_wrap(args.first), # yieldarg
+        Primitive.cext_wrap(value), # procarg,
+        args.size, # argc
+        Truffle::CExt.RARRAY_PTR(args), # argv
+        Primitive.cext_wrap(block), # blockarg
+      ], Primitive.caller_special_variables_if_available, nil)
     end
   end
 
@@ -1227,7 +1226,7 @@ def rb_enumeratorize(obj, meth, args)
   def rb_enumeratorize_with_size(obj, meth, args, size_fn)
     return rb_enumeratorize(obj, meth, args) if size_fn.nil?
     enum = obj.to_enum(meth, *args) do
-      Primitive.cext_unwrap(Primitive.call_with_c_mutex_and_frame(size_fn, [Primitive.cext_wrap(obj), Primitive.cext_wrap(args), Primitive.cext_wrap(enum)], Primitive.caller_special_variables_if_available, nil))
+      Primitive.call_with_c_mutex_and_frame_and_unwrap(size_fn, [Primitive.cext_wrap(obj), Primitive.cext_wrap(args), Primitive.cext_wrap(enum)], Primitive.caller_special_variables_if_available, nil)
     end
     enum
   end
@@ -1242,7 +1241,7 @@ def rb_newobj_of(ruby_class)
 
   def rb_define_alloc_func(ruby_class, function)
     ruby_class.singleton_class.define_method(:__allocate__) do
-      Primitive.cext_unwrap(Primitive.call_with_c_mutex_and_frame(function, [Primitive.cext_wrap(self)], Primitive.caller_special_variables_if_available, nil))
+      Primitive.call_with_c_mutex_and_frame_and_unwrap(function, [Primitive.cext_wrap(self)], Primitive.caller_special_variables_if_available, nil)
     end
     class << ruby_class
       private :__allocate__
@@ -1667,13 +1666,13 @@ def rb_thread_call_without_gvl(function, data1, unblock, data2)
   def rb_iterate(iteration, iterated_object, callback, callback_arg)
     block = rb_block_proc
     wrapped_callback = proc do |block_arg|
-      Primitive.cext_unwrap(Primitive.call_with_c_mutex_and_frame(callback, [
+      Primitive.call_with_c_mutex_and_frame_and_unwrap(callback, [
         Primitive.cext_wrap(block_arg),
         Primitive.cext_wrap(callback_arg),
         0, # argc
         nil, # argv
         nil, # blockarg
-      ], Primitive.cext_special_variables_from_stack, block))
+      ], Primitive.cext_special_variables_from_stack, block)
     end
     Primitive.cext_unwrap(
       Primitive.call_with_c_mutex_and_frame(iteration, [Primitive.cext_wrap(iterated_object)], Primitive.cext_special_variables_from_stack, wrapped_callback))
@@ -1785,7 +1784,7 @@ def rb_define_hooked_variable(name, gvar, getter, setter)
     id = name.to_sym
 
     getter_proc = -> {
-      Primitive.cext_unwrap(Primitive.call_with_c_mutex_and_frame(getter, [Primitive.cext_wrap(id), gvar, Primitive.cext_wrap(nil)], Primitive.caller_special_variables_if_available, nil))
+      Primitive.call_with_c_mutex_and_frame_and_unwrap(getter, [Primitive.cext_wrap(id), gvar, Primitive.cext_wrap(nil)], Primitive.caller_special_variables_if_available, nil)
     }
 
     setter_proc = -> value {
@@ -1910,14 +1909,13 @@ def rb_fiber_current
 
   def rb_fiber_new(function, value)
     Fiber.new do |*args|
-      Primitive.cext_unwrap(
-        Primitive.call_with_c_mutex_and_frame(function, [
-          Primitive.cext_wrap(args.first), # yieldarg
-          nil, # procarg,
-          0, # argc
-          nil, # argv
-          nil, # blockarg
-        ], nil, nil))
+      Primitive.call_with_c_mutex_and_frame_and_unwrap(function, [
+        Primitive.cext_wrap(args.first), # yieldarg
+        nil, # procarg,
+        0, # argc
+        nil, # argv
+        nil, # blockarg
+      ], nil, nil)
     end
   end
 

lib/truffle/truffle/cext_ruby.rb

@@ -38,7 +38,7 @@ def rb_define_method(mod, name, function, argc)
       # We must set block argument if given here so that the
       # `rb_block_*` functions will be able to find it by walking the
       # stack.
-      res = Primitive.cext_unwrap(Primitive.call_with_c_mutex_and_frame(function, args, Primitive.caller_special_variables_if_available, block))
+      res = Primitive.call_with_c_mutex_and_frame_and_unwrap(function, args, Primitive.caller_special_variables_if_available, block)
       Primitive.thread_set_exception(exc)
       res
     end

src/main/java/org/truffleruby/cext/CExtNodes.java

@@ -159,6 +159,55 @@ protected Object callWithCExtLockAndFrame(
         }
     }
 
+    @Primitive(name = "call_with_c_mutex_and_frame_and_unwrap")
+    public abstract static class CallWithCExtLockAndFrameAndUnwrapNode extends PrimitiveArrayArgumentsNode {
+
+        @Specialization(limit = "getCacheLimit()")
+        protected Object callWithCExtLockAndFrame(
+                VirtualFrame frame, Object receiver, RubyArray argsArray, Object specialVariables, Object block,
+                @CachedLibrary("receiver") InteropLibrary receivers,
+                @Cached ArrayToObjectArrayNode arrayToObjectArrayNode,
+                @Cached TranslateInteropExceptionNode translateInteropExceptionNode,
+                @Cached ConditionProfile ownedProfile,
+                @Cached UnwrapNode unwrapNode) {
+            final ExtensionCallStack extensionStack = getLanguage()
+                    .getCurrentThread()
+                    .getCurrentFiber().extensionCallStack;
+            final boolean keywordsGiven = RubyArguments.getDescriptor(frame) instanceof KeywordArgumentsDescriptor;
+            extensionStack.push(keywordsGiven, specialVariables, block);
+            try {
+                final Object[] args = arrayToObjectArrayNode.executeToObjectArray(argsArray);
+
+                if (getContext().getOptions().CEXT_LOCK) {
+                    final ReentrantLock lock = getContext().getCExtensionsLock();
+                    boolean owned = ownedProfile.profile(lock.isHeldByCurrentThread());
+
+                    if (!owned) {
+                        MutexOperations.lockInternal(getContext(), lock, this);
+                    }
+                    try {
+                        return unwrapNode.execute(
+                                InteropNodes.execute(receiver, args, receivers, translateInteropExceptionNode));
+                    } finally {
+                        if (!owned) {
+                            MutexOperations.unlockInternal(lock);
+                        }
+                    }
+                } else {
+                    return unwrapNode
+                            .execute(InteropNodes.execute(receiver, args, receivers, translateInteropExceptionNode));
+                }
+
+            } finally {
+                extensionStack.pop();
+            }
+        }
+
+        protected int getCacheLimit() {
+            return getLanguage().options.DISPATCH_CACHE;
+        }
+    }
+
     @Primitive(name = "call_with_c_mutex")
     public abstract static class CallWithCExtLockNode extends PrimitiveArrayArgumentsNode {