Copy progenitor vartable manager when extending Data

headius · headius · commit 4e6a0b9d1552 · 2025-04-01T21:29:47.000-05:00
We walk up the hierarchy to get the actual Data definitions, but
each concrete allocatable class should have its own variable table
manager.
diff --git a/core/src/main/java/org/jruby/RubyClass.java b/core/src/main/java/org/jruby/RubyClass.java
@@ -555,6 +555,7 @@ public static RubyClass newClass(ThreadContext context, RubyClass superClass, St
                 baseName(name);
         clazz.makeMetaClass(context, superClass.getMetaClass());
         if (setParent) clazz.setParent(parent);
+        clazz.copyVariableTableManager(context, superClass);
         parent.setConstant(context, name, clazz, file, line);
         superClass.invokeInherited(context, superClass, clazz);
         return clazz;
@@ -1077,6 +1078,7 @@ private RubyClass initializeCommon(ThreadContext context, RubyClass superClazz,
         allocator = superClazz.allocator;
         makeMetaClass(context, superClazz.getMetaClass());
         superClazz.addSubclass(this);
+        copyVariableTableManager(context, superClazz);
 
         marshal = superClazz.marshal;
 
@@ -1086,6 +1088,14 @@ private RubyClass initializeCommon(ThreadContext context, RubyClass superClazz,
         return this;
     }
 
+    private void copyVariableTableManager(ThreadContext context, RubyClass superClazz) {
+        VariableTableManager variableTableManager = superClazz.getVariableTableManager();
+        if (variableTableManager.getRealClass().superClass() == context.runtime.getData()) {
+            // duplicate data's variable table in subclasses
+            this.variableTableManager = variableTableManager.duplicate();
+        }
+    }
+
     /** rb_class_init_copy
      *
      */
@@ -3484,5 +3494,5 @@ private CallSite[] createCallSites() {
     private final RubyClass realClass;
 
     /** Variable table manager for this class */
-    private final VariableTableManager variableTableManager;
+    private VariableTableManager variableTableManager;
 }
diff --git a/core/src/main/java/org/jruby/runtime/ivars/VariableTableManager.java b/core/src/main/java/org/jruby/runtime/ivars/VariableTableManager.java
@@ -32,6 +32,7 @@
 import java.io.ObjectOutputStream;
 import java.lang.invoke.MethodHandle;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.Map;
@@ -105,6 +106,27 @@ public VariableTableManager(RubyClass realClass) {
         this.realClass = realClass;
     }
 
+    /**
+     * Copy constructor with deep cloning.
+     *
+     * @param original VariableTableManager to copy
+     */
+    VariableTableManager(VariableTableManager original) {
+        synchronized (original) {
+            this.realClass = original.realClass;
+            this.variableAccessors = copyVariableAccessors(original.variableAccessors);
+            this.variableNames = original.variableNames.clone();
+            this.hasObjectID = original.hasObjectID;
+            this.hasFFI = original.hasFFI;
+            this.hasObjectspaceGroup = original.hasObjectspaceGroup;
+            this.fieldVariables = original.fieldVariables;
+        }
+    }
+
+    public RubyClass getRealClass() {
+        return realClass;
+    }
+
     /**
      * Get the map of all current variable accessors with intent to read from it.
      *
@@ -253,18 +275,24 @@ VariableAccessor getVariableAccessorWithBuilder(String name, Function<Integer, V
                 if (ivarAccessor == null) {
                     // allocate a new accessor and populate a new table
                     ivarAccessor = allocateVariableAccessors(name, defaultAccessorBuilder);
-                    Map<String, VariableAccessor> newVariableAccessors = new LinkedHashMap<>(myVariableAccessors.size() + 1);
-
-                    newVariableAccessors.putAll(myVariableAccessors);
-                    newVariableAccessors.put(name, ivarAccessor);
-
-                    variableAccessors = newVariableAccessors;
+                    variableAccessors = copyVariableAccessors(myVariableAccessors, name, ivarAccessor);
                 }
             }
         }
         return ivarAccessor;
     }
 
+    private static Map<String, VariableAccessor> copyVariableAccessors(Map<String, VariableAccessor> myVariableAccessors) {
+        return new LinkedHashMap<>(myVariableAccessors);
+    }
+
+    private static Map<String, VariableAccessor> copyVariableAccessors(Map<String, VariableAccessor> myVariableAccessors, String name, VariableAccessor ivarAccessor) {
+        LinkedHashMap<String, VariableAccessor> newVariableAccessors = new LinkedHashMap<>(myVariableAccessors.size() + 1);
+        newVariableAccessors.putAll(myVariableAccessors);
+        newVariableAccessors.put(name, ivarAccessor);
+        return newVariableAccessors;
+    }
+
     /**
      * Get the variable accessor for the given name with intent to use it for
      * reading.
@@ -514,6 +542,10 @@ public Object clearVariable(RubyBasicObject object, String name) {
         }
     }
 
+    public VariableTableManager duplicate() {
+        return new VariableTableManager(this);
+    }
+
     /**
      * We lazily stand up the object ID since it forces us to stand up
      * per-object state for a given object. We also check for ObjectSpace here,
