https://git.ruby-lang.org/ruby.git/commit/?id=3b6954f8b9

From 3b6954f8b9189f599e1f17636f8667a95ee94193 Mon Sep 17 00:00:00 2001
From: John Hawthorn <john@h...>
Date: Thu, 7 Nov 2019 14:50:05 -0800
Subject: Fix passing actual object_id to finalizer

Previously we were passing the memory_id. This was broken previously if
compaction was run (which changes the memory_id) and now that object_id
is a monotonically increasing number it was always broken.

This commit fixes this by defering removal from the object_id table
until finalizers have run (for objects with finalizers) and also copying
the SEEN_OBJ_ID flag onto the zombie objects.

diff --git a/gc.c b/gc.c
index 7fd80a3..2ef9569 100644
--- a/gc.c
+++ b/gc.c
@@ -2484,7 +2484,7 @@ static inline void https://github.com/ruby/ruby/blob/trunk/gc.c#L2484
 make_zombie(rb_objspace_t *objspace, VALUE obj, void (*dfree)(void *), void *data)
 {
     struct RZombie *zombie = RZOMBIE(obj);
-    zombie->basic.flags = T_ZOMBIE;
+    zombie->basic.flags = T_ZOMBIE | (zombie->basic.flags & FL_SEEN_OBJ_ID);
     zombie->dfree = dfree;
     zombie->data = data;
     zombie->next = heap_pages_deferred_final;
@@ -2498,6 +2498,23 @@ make_io_zombie(rb_objspace_t *objspace, VALUE obj) https://github.com/ruby/ruby/blob/trunk/gc.c#L2498
     make_zombie(objspace, obj, (void (*)(void*))rb_io_fptr_finalize, fptr);
 }
 
+static void
+obj_free_object_id(rb_objspace_t *objspace, VALUE obj)
+{
+    VALUE id;
+
+    GC_ASSERT(FL_TEST(obj, FL_SEEN_OBJ_ID));
+    FL_UNSET(obj, FL_SEEN_OBJ_ID);
+
+    if (st_delete(objspace->obj_to_id_tbl, (st_data_t *)&obj, &id)) {
+        GC_ASSERT(id);
+        st_delete(objspace->id_to_obj_tbl, (st_data_t *)&id, NULL);
+    }
+    else {
+        rb_bug("Object ID seen, but not in mapping table: %s\n", obj_info(obj));
+    }
+}
+
 static int
 obj_free(rb_objspace_t *objspace, VALUE obj)
 {
@@ -2519,18 +2536,8 @@ obj_free(rb_objspace_t *objspace, VALUE obj) https://github.com/ruby/ruby/blob/trunk/gc.c#L2536
 	FL_UNSET(obj, FL_EXIVAR);
     }
 
-    if (FL_TEST(obj, FL_SEEN_OBJ_ID)) {
-        VALUE id;
-
-        FL_UNSET(obj, FL_SEEN_OBJ_ID);
-
-        if (st_delete(objspace->obj_to_id_tbl, (st_data_t *)&obj, &id)) {
-            GC_ASSERT(id);
-            st_delete(objspace->id_to_obj_tbl, (st_data_t *)&id, NULL);
-        }
-        else {
-            rb_bug("Object ID see, but not in mapping table: %s\n", obj_info(obj));
-        }
+    if (FL_TEST(obj, FL_SEEN_OBJ_ID) && !FL_TEST(obj, FL_FINALIZE)) {
+        obj_free_object_id(objspace, obj);
     }
 
 #if USE_RGENGC
@@ -3308,7 +3315,7 @@ run_finalizer(rb_objspace_t *objspace, VALUE obj, VALUE table) https://github.com/ruby/ruby/blob/trunk/gc.c#L3315
 
     saved.safe = rb_safe_level();
     saved.errinfo = rb_errinfo();
-    saved.objid = nonspecial_obj_id(obj);
+    saved.objid = rb_obj_id(obj);
     saved.cfp = ec->cfp;
     saved.finished = 0;
 
@@ -3353,6 +3360,11 @@ finalize_list(rb_objspace_t *objspace, VALUE zombie) https://github.com/ruby/ruby/blob/trunk/gc.c#L3360
 
 	run_final(objspace, zombie);
 
+        GC_ASSERT(BUILTIN_TYPE(zombie) == T_ZOMBIE);
+        if (FL_TEST(zombie, FL_SEEN_OBJ_ID)) {
+            obj_free_object_id(objspace, zombie);
+        }
+
 	RZOMBIE(zombie)->basic.flags = 0;
         if (LIKELY(heap_pages_final_slots)) heap_pages_final_slots--;
 	page->final_slots--;
@@ -3595,7 +3607,7 @@ rb_objspace_garbage_object_p(VALUE obj) https://github.com/ruby/ruby/blob/trunk/gc.c#L3607
  */
 
 static VALUE
-id2ref(VALUE obj, VALUE objid)
+id2ref(VALUE objid)
 {
 #if SIZEOF_LONG == SIZEOF_VOIDP
 #define NUM2PTR(x) NUM2ULONG(x)
@@ -3637,6 +3649,12 @@ id2ref(VALUE obj, VALUE objid) https://github.com/ruby/ruby/blob/trunk/gc.c#L3649
 }
 
 static VALUE
+os_id2ref(VALUE os, VALUE objid)
+{
+    return id2ref(objid);
+}
+
+static VALUE
 rb_find_object_id(VALUE obj, VALUE (*get_heap_object_id)(VALUE))
 {
     if (STATIC_SYM_P(obj)) {
@@ -5946,7 +5964,7 @@ verify_internal_consistency_i(void *page_start, void *page_end, size_t stride, v https://github.com/ruby/ruby/blob/trunk/gc.c#L5964
 	}
 	else {
 	    if (BUILTIN_TYPE(obj) == T_ZOMBIE) {
-		GC_ASSERT(RBASIC(obj)->flags == T_ZOMBIE);
+		GC_ASSERT((RBASIC(obj)->flags & ~FL_SEEN_OBJ_ID) == T_ZOMBIE);
 		data->zombie_object_count++;
 	    }
 	}
@@ -10429,7 +10447,7 @@ wmap_finalize(RB_BLOCK_CALL_FUNC_ARGLIST(objid, self)) https://github.com/ruby/ruby/blob/trunk/gc.c#L10447
 
     TypedData_Get_Struct(self, struct weakmap, &weakmap_type, w);
     /* Get reference from object id. */
-    obj = obj_id_to_ref(objid);
+    obj = id2ref(objid);
 
     /* obj is original referenced object and/or weak reference. */
     orig = (st_data_t)obj;
@@ -11840,7 +11858,7 @@ Init_GC(void) https://github.com/ruby/ruby/blob/trunk/gc.c#L11858
     rb_define_module_function(rb_mObjSpace, "define_finalizer", define_final, -1);
     rb_define_module_function(rb_mObjSpace, "undefine_finalizer", undefine_final, 1);
 
-    rb_define_module_function(rb_mObjSpace, "_id2ref", id2ref, 1);
+    rb_define_module_function(rb_mObjSpace, "_id2ref", os_id2ref, 1);
 
     rb_vm_register_special_exception(ruby_error_nomemory, rb_eNoMemError, "failed to allocate memory");
 
diff --git a/test/ruby/test_gc.rb b/test/ruby/test_gc.rb
index f42e098..ef99f69 100644
--- a/test/ruby/test_gc.rb
+++ b/test/ruby/test_gc.rb
@@ -372,6 +372,14 @@ class TestGc < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_gc.rb#L372
     assert_empty(out)
   end
 
+  def test_finalizer_passed_object_id
+    assert_in_out_err(%w[--disable-gems], <<-EOS, ["true"], [])
+      o = Object.new
+      obj_id = o.object_id
+      ObjectSpace.define_finalizer(o, ->(id){ p id == obj_id })
+    EOS
+  end
+
   def test_verify_internal_consistency
     assert_nil(GC.verify_internal_consistency)
   end
-- 
cgit v0.10.2


--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/