jruby · Sep 1, 2015 · Sep 1, 2015
diff --git a/spec/ruby/core/objectspace/each_object_spec.rb b/spec/ruby/core/objectspace/each_object_spec.rb
@@ -1,8 +1,6 @@
 require File.expand_path('../../../spec_helper', __FILE__)
 require File.expand_path('../fixtures', __FILE__)
 
-require 'weakref'
-
 describe "ObjectSpace.each_object" do
   it "calls the block once for each living, non-immediate object in the Ruby process" do
     class ObjectSpaceSpecEachObject; end
@@ -49,20 +47,20 @@ class ObjectSpaceSpecEachOtherObject; end
 
   it "finds an object stored in a global variable" do
     $object_space_global_variable = ObjectSpaceFixtures::ObjectToBeFound.new(:global)
-    ObjectSpaceFixtures::to_be_found_symbols.should include(:global)
+    ObjectSpaceFixtures.to_be_found_symbols.should include(:global)
   end
 
   it "finds an object stored in a top-level constant" do
-    ObjectSpaceFixtures::to_be_found_symbols.should include(:top_level_constant)
+    ObjectSpaceFixtures.to_be_found_symbols.should include(:top_level_constant)
   end
 
   it "finds an object stored in a second-level constant" do
-    ObjectSpaceFixtures::to_be_found_symbols.should include(:second_level_constant)
+    ObjectSpaceFixtures.to_be_found_symbols.should include(:second_level_constant)
   end
 
   it "finds an object stored in a local variable" do
     local = ObjectSpaceFixtures::ObjectToBeFound.new(:local)
-    ObjectSpaceFixtures::to_be_found_symbols.should include(:local)
+    ObjectSpaceFixtures.to_be_found_symbols.should include(:local)
   end
 
   it "finds an object stored in a local variable captured in a block explicitly" do
@@ -71,7 +69,7 @@ class ObjectSpaceSpecEachOtherObject; end
       Proc.new { local_in_block }
     }.call
 
-    ObjectSpaceFixtures::to_be_found_symbols.should include(:local_in_block_explicit)
+    ObjectSpaceFixtures.to_be_found_symbols.should include(:local_in_block_explicit)
   end
 
   it "finds an object stored in a local variable captured in a block implicitly" do
@@ -80,7 +78,7 @@ class ObjectSpaceSpecEachOtherObject; end
       Proc.new { }
     }.call
 
-    ObjectSpaceFixtures::to_be_found_symbols.should include(:local_in_block_implicit)
+    ObjectSpaceFixtures.to_be_found_symbols.should include(:local_in_block_implicit)
   end
 
   it "finds an object stored in a local variable captured in a Proc#binding" do
@@ -89,7 +87,7 @@ class ObjectSpaceSpecEachOtherObject; end
       Proc.new { }.binding
     }.call
 
-    ObjectSpaceFixtures::to_be_found_symbols.should include(:local_in_proc_binding)
+    ObjectSpaceFixtures.to_be_found_symbols.should include(:local_in_proc_binding)
   end
 
   it "finds an object stored in a local variable captured in a Kernel#binding" do
@@ -98,48 +96,51 @@ class ObjectSpaceSpecEachOtherObject; end
       binding
     }.call
 
-    ObjectSpaceFixtures::to_be_found_symbols.should include(:local_in_kernel_binding)
+    ObjectSpaceFixtures.to_be_found_symbols.should include(:local_in_kernel_binding)
   end
 
   it "finds an object stored in a local variable set in a binding manually" do
     b = binding
     b.local_variable_set(:local, ObjectSpaceFixtures::ObjectToBeFound.new(:local_in_manual_binding))
-    ObjectSpaceFixtures::to_be_found_symbols.should include(:local_in_manual_binding)
+    ObjectSpaceFixtures.to_be_found_symbols.should include(:local_in_manual_binding)
   end
 
   it "finds an object stored in an array" do
     array = [ObjectSpaceFixtures::ObjectToBeFound.new(:array)]
-    ObjectSpaceFixtures::to_be_found_symbols.should include(:array)
+    ObjectSpaceFixtures.to_be_found_symbols.should include(:array)
   end
 
   it "finds an object stored in a hash key" do
     hash = {ObjectSpaceFixtures::ObjectToBeFound.new(:hash_key) => :value}
-    ObjectSpaceFixtures::to_be_found_symbols.should include(:hash_key)
+    ObjectSpaceFixtures.to_be_found_symbols.should include(:hash_key)
   end
 
   it "finds an object stored in a hash value" do
     hash = {a: ObjectSpaceFixtures::ObjectToBeFound.new(:hash_value)}
-    ObjectSpaceFixtures::to_be_found_symbols.should include(:hash_value)
+    ObjectSpaceFixtures.to_be_found_symbols.should include(:hash_value)
   end
 
   it "finds an object stored in an instance variable" do
     local = ObjectSpaceFixtures::ObjectWithInstanceVariable.new
-    ObjectSpaceFixtures::to_be_found_symbols.should include(:instance_variable)
+    ObjectSpaceFixtures.to_be_found_symbols.should include(:instance_variable)
   end
 
   it "doesn't find an object stored in a WeakRef that should have been cleared" do
+    require 'weakref'
+
     weak_ref = WeakRef.new(ObjectSpaceFixtures::ObjectToBeFound.new(:weakref))
-    ObjectSpaceFixtures::to_be_found_symbols.should_not include(:weakref)
+    ObjectSpaceFixtures.wait_for_weakref_cleared(weak_ref)
+    ObjectSpaceFixtures.to_be_found_symbols.should_not include(:weakref)
   end
 
   it "finds an object stored in a thread local" do
     Thread.current.thread_variable_set(:object_space_thread_local, ObjectSpaceFixtures::ObjectToBeFound.new(:thread_local))
-    ObjectSpaceFixtures::to_be_found_symbols.should include(:thread_local)
+    ObjectSpaceFixtures.to_be_found_symbols.should include(:thread_local)
   end
 
   it "finds an object stored in a fiber local" do
     Thread.current[:object_space_fiber_local] = ObjectSpaceFixtures::ObjectToBeFound.new(:fiber_local)
-    ObjectSpaceFixtures::to_be_found_symbols.should include(:fiber_local)
+    ObjectSpaceFixtures.to_be_found_symbols.should include(:fiber_local)
   end
 
   it "finds an object captured in an at_exit handler" do
@@ -151,21 +152,21 @@ class ObjectSpaceSpecEachOtherObject; end
       end
     }.call
 
-    ObjectSpaceFixtures::to_be_found_symbols.should include(:at_exit)
+    ObjectSpaceFixtures.to_be_found_symbols.should include(:at_exit)
   end
 
   it "finds an object captured in finalizer" do
     alive = Object.new
 
     Proc.new {
-      local = ObjectSpaceFixtures::ObjectToBeFound.new(:at_exit)
+      local = ObjectSpaceFixtures::ObjectToBeFound.new(:finalizer)
 
       ObjectSpace.define_finalizer(alive, Proc.new {
         local
       })
     }.call
 
-    ObjectSpaceFixtures::to_be_found_symbols.should include(:at_exit)
+    ObjectSpaceFixtures.to_be_found_symbols.should include(:finalizer)
 
     alive.should_not be_nil
   end

diff --git a/spec/ruby/core/objectspace/fixtures.rb b/spec/ruby/core/objectspace/fixtures.rb
@@ -33,10 +33,10 @@ def self.scoped(wr)
   end
 
   class ObjectToBeFound
-    attr_reader :id
+    attr_reader :name
 
-    def initialize(id)
-      @id = id
+    def initialize(name)
+      @name = name
     end
   end
 
@@ -47,9 +47,16 @@ def initialize
   end
 
   def self.to_be_found_symbols
-    GC.start
     ObjectSpace.each_object(ObjectToBeFound).map do |o|
-      o.id
+      o.name
+    end
+  end
+
+  def self.wait_for_weakref_cleared(weakref)
+    started = Time.now
+
+    while weakref.weakref_alive? && Time.now - started < 3
+      GC.start
     end
   end
 

diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/core/ObjectSpaceNodes.java b/truffle/src/main/java/org/jruby/truffle/nodes/core/ObjectSpaceNodes.java
@@ -165,9 +165,9 @@ public DefineFinalizerNode(RubyContext context, SourceSection sourceSection) {
         }
 
         @Specialization
-        public DynamicObject defineFinalizer(VirtualFrame frame, Object object, Object finalizer) {
+        public DynamicObject defineFinalizer(VirtualFrame frame, DynamicObject object, Object finalizer) {
             if (respondToNode.executeBoolean(frame, finalizer)) {
-                registerFinalizer(object, finalizer);
+                getContext().getObjectSpaceManager().defineFinalizer(object, finalizer);
                 Object[] objects = new Object[]{0, finalizer};
                 return Layouts.ARRAY.createArray(getContext().getCoreLibrary().getArrayFactory(), objects, objects.length);
             } else {
@@ -176,10 +176,6 @@ public DynamicObject defineFinalizer(VirtualFrame frame, Object object, Object f
             }
         }
 
-        @TruffleBoundary
-        private void registerFinalizer(Object object, Object finalizer) {
-            getContext().getObjectSpaceManager().defineFinalizer((DynamicObject) object, finalizer);
-        }
     }
 
     @CoreMethod(names = "undefine_finalizer", isModuleFunction = true, required = 1)

diff --git a/truffle/src/main/java/org/jruby/truffle/runtime/object/ObjectGraph.java b/truffle/src/main/java/org/jruby/truffle/runtime/object/ObjectGraph.java
@@ -87,6 +87,10 @@ public void run(DynamicObject thread, Node currentNode) {
                             for (DynamicObject handler : context.getAtExitManager().getHandlers()) {
                                 visitObject(handler, visitor);
                             }
+
+                            for (DynamicObject handler : context.getObjectSpaceManager().getFinalizerHandlers()) {
+                                visitObject(handler, visitor);
+                            }
                         }
 
                         // All threads visit their thread object
@@ -136,16 +140,20 @@ private void visitObject(DynamicObject object, ObjectGraphVisitor visitor) throw
             // Visit specific objects that we're managing
 
             if (RubyGuards.isRubyModule(object)) {
-                final ModuleFields module = Layouts.MODULE.getFields(object);
+                visitModule(object, visitor);
+            }
+        }
+    }
 
-                for (DynamicObject ancestor : module.ancestors()) {
-                    visitObject(ancestor, visitor);
-                }
+    private void visitModule(DynamicObject module, ObjectGraphVisitor visitor) {
+        final ModuleFields fields = Layouts.MODULE.getFields(module);
 
-                for (RubyConstant constant : module.getConstants().values()) {
-                    visitObject(constant.getValue(), visitor);
-                }
-            }
+        for (DynamicObject ancestor : fields.ancestors()) {
+            visitObject(ancestor, visitor);
+        }
+
+        for (RubyConstant constant : fields.getConstants().values()) {
+            visitObject(constant.getValue(), visitor);
         }
     }
 

diff --git a/truffle/src/main/java/org/jruby/truffle/runtime/subsystems/ObjectSpaceManager.java b/truffle/src/main/java/org/jruby/truffle/runtime/subsystems/ObjectSpaceManager.java
@@ -9,6 +9,7 @@
  */
 package org.jruby.truffle.runtime.subsystems;
 
+import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.object.DynamicObject;
 import org.jruby.truffle.nodes.core.ThreadNodes;
 import org.jruby.truffle.runtime.RubyContext;
@@ -17,10 +18,7 @@
 
 import java.lang.ref.ReferenceQueue;
 import java.lang.ref.WeakReference;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Map;
-import java.util.WeakHashMap;
+import java.util.*;
 
 /**
  * Supports the Ruby {@code ObjectSpace} module. Object IDs are lazily allocated {@code long}
@@ -61,6 +59,7 @@ public ObjectSpaceManager(RubyContext context) {
         this.context = context;
     }
 
+    @CompilerDirectives.TruffleBoundary
     public synchronized void defineFinalizer(DynamicObject object, Object callable) {
         // Record the finalizer against the object
 
@@ -122,4 +121,18 @@ private static void runFinalizers(RubyContext context, FinalizerReference finali
         }
     }
 
+    public List<DynamicObject> getFinalizerHandlers() {
+        final List<DynamicObject> handlers = new ArrayList<>();
+
+        for (FinalizerReference finalizer : finalizerReferences.values()) {
+            for (Object handler : finalizer.getFinalizers()) {
+                if (handler instanceof DynamicObject) {
+                    handlers.add((DynamicObject) handler);
+                }
+            }
+        }
+
+        return handlers;
+    }
+
 }