diff --git a/CHANGELOG.md b/CHANGELOG.md index b24fc38e2897..a349e397a81c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ New features: Bug fixes: +* Fix `rb_gc_register_address()`/`rb_global_variable()` to read the latest value (#2721, #2734, #2720, @eregon). Compatibility: diff --git a/lib/truffle/truffle/cext.rb b/lib/truffle/truffle/cext.rb index 3d4d183b7365..2d717addb4c7 100644 --- a/lib/truffle/truffle/cext.rb +++ b/lib/truffle/truffle/cext.rb @@ -108,8 +108,6 @@ module Truffle::CExt RUBY_ECONV_PARTIAL_INPUT = Encoding::Converter::PARTIAL_INPUT RUBY_ECONV_AFTER_OUTPUT = Encoding::Converter::AFTER_OUTPUT - GLOBALLY_PRESERVED_VALUES = [] - SET_LIBTRUFFLERUBY = -> libtruffleruby do LIBTRUFFLERUBY = libtruffleruby end @@ -1971,15 +1969,31 @@ def rb_exception_set_message(e, mesg) Primitive.exception_set_message(e, mesg) end - def rb_global_variable(obj) - GLOBALLY_PRESERVED_VALUES << obj - end - + # This could also be an Array similar to the global_list in MRI. + # But that makes it more difficult to test the case the global is in native memory, + # and would be slower for rb_gc_unregister_address(). GC_REGISTERED_ADDRESSES = {} - def rb_gc_register_address(address, obj) + def rb_global_variable(address) + rb_gc_register_address(address, 'rb_global_variable') + end + + def rb_gc_register_address(address, function = 'rb_gc_register_address') + # Make it a native pointer so its identity hash is stable Truffle::Interop.to_native(address) unless Truffle::Interop.pointer?(address) - GC_REGISTERED_ADDRESSES[address] = obj + + c_global_variables = Primitive.fiber_c_global_variables + unless c_global_variables + raise "#{function}() called outside of Init_ function, this is not supported yet on TruffleRuby" + end + c_global_variables << address + end + + def resolve_registered_addresses + c_global_variables = Primitive.fiber_c_global_variables + c_global_variables.each do |address| + GC_REGISTERED_ADDRESSES[address] = LIBTRUFFLERUBY.rb_tr_read_VALUE_pointer(address) + end end def rb_gc_unregister_address(address) diff --git a/src/main/c/cext/gc.c b/src/main/c/cext/gc.c index 6497da497eb6..7742edd63e5c 100644 --- a/src/main/c/cext/gc.c +++ b/src/main/c/cext/gc.c @@ -12,11 +12,14 @@ // GC, rb_gc_* +void rb_global_variable(VALUE *address) { + /* NOTE: this captures the value after the Init_ function returns and assumes the value does not change after that. */ + polyglot_invoke(RUBY_CEXT, "rb_global_variable", address); +} + void rb_gc_register_address(VALUE *address) { - /* TODO: This should guard the address of the pointer, not the - object pointed to, but we haven't yet found a good way to implement - that, or a real world use case where it is required. */ - polyglot_invoke(RUBY_CEXT, "rb_gc_register_address", address, rb_tr_unwrap(*address)); + /* NOTE: this captures the value after the Init_ function returns and assumes the value does not change after that. */ + polyglot_invoke(RUBY_CEXT, "rb_gc_register_address", address); } void rb_gc_unregister_address(VALUE *address) { @@ -58,9 +61,7 @@ void rb_gc_register_mark_object(VALUE obj) { RUBY_CEXT_INVOKE_NO_WRAP("rb_gc_register_mark_object", obj); } -void rb_global_variable(VALUE *obj) { - /* TODO: This should guard the address of the pointer, not the - object pointed to, but we haven't yet found a good way to implement - that, or a real world use case where it is required. */ - RUBY_CEXT_INVOKE_NO_WRAP("rb_global_variable", *obj); +void* rb_tr_read_VALUE_pointer(VALUE *pointer) { + VALUE value = *pointer; + return rb_tr_unwrap(value); } diff --git a/src/main/java/org/truffleruby/core/fiber/FiberNodes.java b/src/main/java/org/truffleruby/core/fiber/FiberNodes.java index d7a7ef0f2103..02a21018ca93 100644 --- a/src/main/java/org/truffleruby/core/fiber/FiberNodes.java +++ b/src/main/java/org/truffleruby/core/fiber/FiberNodes.java @@ -393,4 +393,18 @@ protected Object isBlocking() { } + @Primitive(name = "fiber_c_global_variables") + public abstract static class FiberCGlobalVariablesNode extends PrimitiveArrayArgumentsNode { + @Specialization + protected Object cGlobalVariables() { + RubyFiber currentFiber = getLanguage().getCurrentFiber(); + var cGlobalVariablesDuringInitFunction = currentFiber.cGlobalVariablesDuringInitFunction; + if (cGlobalVariablesDuringInitFunction == null) { + return nil; + } else { + return cGlobalVariablesDuringInitFunction; + } + } + } + } diff --git a/src/main/java/org/truffleruby/core/fiber/RubyFiber.java b/src/main/java/org/truffleruby/core/fiber/RubyFiber.java index 07e291f750e6..fd13aad1d646 100644 --- a/src/main/java/org/truffleruby/core/fiber/RubyFiber.java +++ b/src/main/java/org/truffleruby/core/fiber/RubyFiber.java @@ -90,6 +90,7 @@ public enum FiberStatus { public final MarkingService.ExtensionCallStack extensionCallStack; public final ValueWrapperManager.HandleBlockHolder handleData; boolean blocking = true; + public RubyArray cGlobalVariablesDuringInitFunction; public RubyFiber( RubyClass rubyClass, diff --git a/src/main/java/org/truffleruby/language/loader/RequireNode.java b/src/main/java/org/truffleruby/language/loader/RequireNode.java index 6da12391862e..9df72f673fb3 100644 --- a/src/main/java/org/truffleruby/language/loader/RequireNode.java +++ b/src/main/java/org/truffleruby/language/loader/RequireNode.java @@ -285,7 +285,11 @@ private void requireCExtension(String feature, String expandedPath, Node current requireMetric("before-execute-" + feature); ValueWrapperManager.allocateNewBlock(getContext(), getLanguage()); - getLanguage().getCurrentFiber().extensionCallStack.push(false, nil, nil); + var currentFiber = getLanguage().getCurrentFiber(); + + var prevGlobals = currentFiber.cGlobalVariablesDuringInitFunction; + currentFiber.cGlobalVariablesDuringInitFunction = createEmptyArray(); + currentFiber.extensionCallStack.push(false, nil, nil); try { InteropNodes .execute( @@ -294,9 +298,14 @@ private void requireCExtension(String feature, String expandedPath, Node current initFunctionInteropLibrary, TranslateInteropExceptionNode.getUncached()); } finally { - getLanguage().getCurrentFiber().extensionCallStack.pop(); - ValueWrapperManager.allocateNewBlock(getContext(), getLanguage()); - requireMetric("after-execute-" + feature); + try { + DispatchNode.getUncached().call(coreLibrary().truffleCExtModule, "resolve_registered_addresses"); + } finally { + currentFiber.extensionCallStack.pop(); + currentFiber.cGlobalVariablesDuringInitFunction = prevGlobals; + ValueWrapperManager.allocateNewBlock(getContext(), getLanguage()); + requireMetric("after-execute-" + feature); + } } }