Fix up [Feature #15974]

* Fixed warning condition
* Fixed function signature
* Use ident hash

2 files changed, 40 insertions, 14 deletions

diff --git a/gc.c b/gc.c
index bb6b0d8ea0..2f6983e67c 100644
--- a/gc.c
+++ b/gc.c
@@ -2961,8 +2961,9 @@ struct should_not_capture_data {
 };
 
 static void
-should_not_capture_callback(const VALUE child, struct should_not_capture_data *data)
+should_not_capture_callback(VALUE child, void *dp)
 {
+    struct should_not_capture_data *data = dp;
     if (child == data->obj)
 	data->found = true;
 
@@ -2970,22 +2971,20 @@ should_not_capture_callback(const VALUE child, struct should_not_capture_data *d
 	return;
 
     // Maintain a set of objects already searched, so that we don't follow a cycle
-    VALUE key = rb_obj_id(child);
-    if (rb_hash_has_key(data->set, key))
+    if (rb_hash_lookup2(data->set, child, Qfalse))
 	return;
-    rb_hash_aset(data->set, key, Qtrue);
+    rb_hash_aset(data->set, child, Qtrue);
 
-    rb_objspace_reachable_objects_from(child, (void (*)(unsigned long, void *)) &should_not_capture_callback, (void *)data);
+    rb_objspace_reachable_objects_from(child, should_not_capture_callback, data);
 }
 
 static void
 should_not_capture(VALUE block, VALUE obj)
 {
-    struct should_not_capture_data data;
-    data.obj = obj;
-    data.set = rb_hash_new();
-    data.found = false;
-    rb_objspace_reachable_objects_from(block, (void (*)(unsigned long, void *)) &should_not_capture_callback, (void *)&data);
+    struct should_not_capture_data data = {obj, rb_ident_hash_new()};
+    rb_obj_hide(data.set);
+    rb_objspace_reachable_objects_from(block, should_not_capture_callback, &data);
+    rb_hash_clear(data.set);
     if (data.found)
 	rb_warn("object is reachable from finalizer - it may never be run");
 }
@@ -3019,8 +3018,9 @@ define_final(int argc, VALUE *argv, VALUE os)
 	should_be_callable(block);
     }
 
-    if (ruby_verbose)
+    if (RTEST(ruby_verbose)) {
 	should_not_capture(block, obj);
+    }
 
     return define_final0(obj, block);
 }
diff --git a/test/ruby/test_objectspace.rb b/test/ruby/test_objectspace.rb
index c352b75b70..9007e05206 100644
--- a/test/ruby/test_objectspace.rb
+++ b/test/ruby/test_objectspace.rb
@@ -56,7 +56,7 @@ End
   end
 
   def test_finalizer
-    assert_in_out_err(["-e", <<-END], "", %w(:ok :ok :ok :ok), [])
+    assert_in_out_err(["-W0", "-e", <<-END], "", %w(:ok :ok :ok :ok))
       a = []
       ObjectSpace.define_finalizer(a) { p :ok }
       b = a.dup
@@ -76,8 +76,8 @@ End
       ObjectSpace.define_finalizer([], fin)
       CODE
     end
-    assert_in_out_err([], code[""], ["finalized"])
-    assert_in_out_err([], code["private "], ["finalized"])
+    assert_in_out_err(["-W0"], code[""], ["finalized"])
+    assert_in_out_err(["-W0"], code["private "], ["finalized"])
     c = EnvUtil.labeled_class("C\u{3042}").new
     o = Object.new
     assert_raise_with_message(ArgumentError, /C\u{3042}/) {
@@ -131,6 +131,32 @@ End
     END
   end
 
+  def test_self_referencing_finalizer
+    assert_separately(["-w"], "#{<<~"begin;"}\n#{<<~'end;'}")
+    begin;
+      obj = +"Test"
+      handler = proc {puts "finalized"}
+      assert_warning(/object is reachable from finalizer/) do
+        ObjectSpace.define_finalizer(obj, handler)
+      end
+    end;
+  end
+
+  def test_indirectly_self_referencing_finalizer
+    assert_separately(["-w"], "#{<<~"begin;"}\n#{<<~'end;'}")
+    begin;
+      def scoped(indirect)
+        proc {puts "finalized"}
+      end
+      obj = +"Test"
+      indirect = [obj]
+      handler = scoped(indirect)
+      assert_warning(/object is reachable from finalizer/) do
+        ObjectSpace.define_finalizer(obj, handler)
+      end
+    end;
+  end
+
   def test_each_object
     klass = Class.new
     new_obj = klass.new