From 08e38878b438f4c34d3bf3033deae378942e3cef Mon Sep 17 00:00:00 2001 From: Duncan MacGregor Date: Tue, 1 Mar 2022 18:16:33 +0000 Subject: [PATCH] Unwrap values returned from C before popping the extension stack. --- CHANGELOG.md | 1 + lib/truffle/truffle/cext.rb | 40 +++++++-------- lib/truffle/truffle/cext_ruby.rb | 2 +- .../java/org/truffleruby/cext/CExtNodes.java | 49 +++++++++++++++++++ 4 files changed, 70 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5eedd16f7013..8ab68c95c053 100644 --- a/CHANGELOG.md +++ b/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: diff --git a/lib/truffle/truffle/cext.rb b/lib/truffle/truffle/cext.rb index a6095360d69d..03b06993dcc4 100644 --- a/lib/truffle/truffle/cext.rb +++ b/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 diff --git a/lib/truffle/truffle/cext_ruby.rb b/lib/truffle/truffle/cext_ruby.rb index 44c178677349..fa86d3a0bbe2 100644 --- a/lib/truffle/truffle/cext_ruby.rb +++ b/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 diff --git a/src/main/java/org/truffleruby/cext/CExtNodes.java b/src/main/java/org/truffleruby/cext/CExtNodes.java index b4dd63b88599..f40da414a637 100644 --- a/src/main/java/org/truffleruby/cext/CExtNodes.java +++ b/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 {