Fix rb_gc_register_address()/rb_global_variable() to read the latest …

…value

* By storing the variables addresses and reading them at the end of the
  Init_ function.
* Raise an error if those functions are called outside Init_ functions.
* Fixes oracle#2721

CHANGELOG.md

@@ -7,6 +7,7 @@ Bug fixes:
 
 * Ensure every parse node has a source section and fix the source section for `ensure` (#2758, @eregon).
 * Fix `spawn(..., fd => fd)` on macOS, it did not work due to a macOS bug (@eregon).
+* Fix `rb_gc_register_address()`/`rb_global_variable()` to read the latest value (#2721, #2734, #2720, @eregon).
 
 Compatibility:
 

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)

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);
 }

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;
+            }
+        }
+    }
+
 }

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,

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);
+            }
         }
     }