[GR-18163] Fix ObjectSpace._id2ref for Symbols and frozen String literals (#2358)

PullRequest: truffleruby/2666

Author: eregon

CHANGELOG.md

@@ -28,6 +28,7 @@ Compatibility:
 * Implement `Fiber#raise` (#2338).
 * Update `File.basename` to return new `String` instances (#2343).
 * Allow `Fiber#raise` after `Fiber#transfer` like Ruby 3.0 (#2342).
+* Fix `ObjectSpace._id2ref` for Symbols and frozen String literals (#2358).
 
 Performance:
 

spec/ruby/core/objectspace/_id2ref_spec.rb

@@ -7,16 +7,43 @@
     r.should == s
   end
 
-  it "retrieves an Integer by object_id" do
-    f = 1
-    r = ObjectSpace._id2ref(f.object_id)
-    r.should == f
+  it "retrieves true by object_id" do
+    ObjectSpace._id2ref(true.object_id).should == true
+  end
+
+  it "retrieves false by object_id" do
+    ObjectSpace._id2ref(false.object_id).should == false
+  end
+
+  it "retrieves nil by object_id" do
+    ObjectSpace._id2ref(nil.object_id).should == nil
+  end
+
+  it "retrieves a small Integer by object_id" do
+    ObjectSpace._id2ref(1.object_id).should == 1
+    ObjectSpace._id2ref((-42).object_id).should == -42
+  end
+
+  it "retrieves a large Integer by object_id" do
+    obj = 1 << 88
+    ObjectSpace._id2ref(obj.object_id).should.equal?(obj)
   end
 
   it "retrieves a Symbol by object_id" do
-    s = :sym
-    r = ObjectSpace._id2ref(s.object_id)
-    r.should == s
+    ObjectSpace._id2ref(:sym.object_id).should.equal?(:sym)
+  end
+
+  it "retrieves a String by object_id" do
+    obj = "str"
+    ObjectSpace._id2ref(obj.object_id).should.equal?(obj)
+  end
+
+  it "retrieves a frozen literal String by object_id" do
+    ObjectSpace._id2ref("frozen string literal _id2ref".freeze.object_id).should.equal?("frozen string literal _id2ref".freeze)
+  end
+
+  it "retrieves an Encoding by object_id" do
+    ObjectSpace._id2ref(Encoding::UTF_8.object_id).should.equal?(Encoding::UTF_8)
   end
 
   it 'raises RangeError when an object could not be found' do

src/main/java/org/truffleruby/core/objectspace/ObjectSpaceNodes.java

@@ -25,6 +25,7 @@
 import org.truffleruby.core.proc.RubyProc;
 import org.truffleruby.core.string.StringUtils;
 import org.truffleruby.core.symbol.RubySymbol;
+import org.truffleruby.language.ImmutableRubyObject;
 import org.truffleruby.language.NotProvided;
 import org.truffleruby.language.RubyDynamicObject;
 import org.truffleruby.language.RubyGuards;
@@ -80,8 +81,8 @@ protected Object id2Ref(long id) {
                 long objectID = 0L;
                 if (object instanceof RubyDynamicObject) {
                     objectID = ObjectSpaceManager.readObjectID((RubyDynamicObject) object, objectLibrary);
-                } else if (object instanceof RubySymbol) {
-                    objectID = ((RubySymbol) object).getObjectId();
+                } else if (object instanceof ImmutableRubyObject) {
+                    objectID = ((ImmutableRubyObject) object).getObjectId();
                 }
 
                 if (objectID == id) {

src/main/java/org/truffleruby/core/string/FrozenStringLiterals.java

@@ -17,6 +17,8 @@
 import org.truffleruby.core.rope.Rope;
 import org.truffleruby.core.rope.RopeCache;
 
+import java.util.Collection;
+
 public class FrozenStringLiterals {
 
     private final RopeCache ropeCache;
@@ -44,4 +46,9 @@ public ImmutableRubyString getFrozenStringLiteral(byte[] bytes, Encoding encodin
         }
     }
 
+    @TruffleBoundary
+    public Collection<ImmutableRubyString> allFrozenStrings() {
+        return values.values();
+    }
+
 }

src/main/java/org/truffleruby/core/symbol/SymbolNodes.java

@@ -63,12 +63,11 @@ public abstract class SymbolNodes {
 
     @CoreMethod(names = "all_symbols", onSingleton = true)
     public abstract static class AllSymbolsNode extends CoreMethodArrayArgumentsNode {
-
+        @TruffleBoundary
         @Specialization
         protected RubyArray allSymbols() {
-            return createArray(getLanguage().symbolTable.allSymbols());
+            return createArray(getLanguage().symbolTable.allSymbols().toArray());
         }
-
     }
 
     @CoreMethod(names = { "==", "eql?" }, required = 1)

src/main/java/org/truffleruby/core/symbol/SymbolTable.java

@@ -151,10 +151,9 @@ public static String checkClassVariableName(
     }
 
     @TruffleBoundary
-    public Object[] allSymbols() {
-        final Collection<RubySymbol> allSymbols = symbolMap.values();
+    public Collection<RubySymbol> allSymbols() {
         // allSymbols is a private concrete collection not a view
-        return allSymbols.toArray();
+        return symbolMap.values();
     }
 
 }

src/main/java/org/truffleruby/language/objects/ObjectGraph.java

@@ -17,6 +17,7 @@
 import java.util.Set;
 
 import org.truffleruby.RubyContext;
+import org.truffleruby.RubyLanguage;
 import org.truffleruby.core.proc.RubyProc;
 import org.truffleruby.language.ImmutableRubyObject;
 import org.truffleruby.language.RubyDynamicObject;
@@ -112,6 +113,10 @@ public static Set<Object> stopAndGetRootObjects(String reason, RubyContext conte
     }
 
     public static void visitContextRoots(RubyContext context, Set<Object> roots) {
+        final RubyLanguage language = context.getLanguageSlow();
+        roots.addAll(language.symbolTable.allSymbols());
+        roots.addAll(language.frozenStringLiterals.allFrozenStrings());
+
         // We do not want to expose the global object
         roots.addAll(context.getCoreLibrary().globalVariables.objectGraphValues());
         roots.addAll(context.getAtExitManager().getHandlers());