From 61d5fb213d48a329e9978dc0c5deb4c5f30c1068 Mon Sep 17 00:00:00 2001
From: Alan Wu <XrXr@users.noreply.github.com>
Date: Tue, 2 Sep 2025 17:42:49 -0400
Subject: [PATCH] ext/-test-/tracepoint/gc_hook.c: Fix GC safety issue

TestTracepointObj#test_teardown_with_active_GC_end_hook was failing on
some platforms due to a Proc that is not marked being passed around.
Neither rb_tracepoint_new() nor rb_postponed_job_preregister() promise
to mark their callback `void *data`.

https://rubyci.s3.amazonaws.com/osx1300arm/ruby-master/log/20250902T154504Z.fail.html.gz

Add a GC.start to make the test a better detector for this safety issue
and fix it by getting the Proc from an ivar on the rooted module.
---
 ext/-test-/tracepoint/gc_hook.c          | 26 +++++++++++++-----------
 ext/-test-/tracepoint/tracepoint.c       | 10 +++++----
 test/-ext-/tracepoint/test_tracepoint.rb |  2 +-
 3 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/ext/-test-/tracepoint/gc_hook.c b/ext/-test-/tracepoint/gc_hook.c
index 54c06c54a5..525be6da63 100644
--- a/ext/-test-/tracepoint/gc_hook.c
+++ b/ext/-test-/tracepoint/gc_hook.c
@@ -2,6 +2,7 @@
 #include "ruby/debug.h"
 
 static int invoking; /* TODO: should not be global variable */
+extern VALUE tp_mBug;
 
 static VALUE
 invoke_proc_ensure(VALUE _)
@@ -17,9 +18,9 @@ invoke_proc_begin(VALUE proc)
 }
 
 static void
-invoke_proc(void *data)
+invoke_proc(void *ivar_name)
 {
-    VALUE proc = (VALUE)data;
+    VALUE proc = rb_ivar_get(tp_mBug, rb_intern(ivar_name));
     invoking += 1;
     rb_ensure(invoke_proc_begin, proc, invoke_proc_ensure, 0);
 }
@@ -40,16 +41,16 @@ gc_start_end_i(VALUE tpval, void *data)
 }
 
 static VALUE
-set_gc_hook(VALUE module, VALUE proc, rb_event_flag_t event, const char *tp_str, const char *proc_str)
+set_gc_hook(VALUE proc, rb_event_flag_t event, const char *tp_str, const char *proc_str)
 {
     VALUE tpval;
     ID tp_key = rb_intern(tp_str);
 
     /* disable previous keys */
-    if (rb_ivar_defined(module, tp_key) != 0 &&
-        RTEST(tpval = rb_ivar_get(module, tp_key))) {
+    if (rb_ivar_defined(tp_mBug, tp_key) != 0 &&
+        RTEST(tpval = rb_ivar_get(tp_mBug, tp_key))) {
         rb_tracepoint_disable(tpval);
-        rb_ivar_set(module, tp_key, Qnil);
+        rb_ivar_set(tp_mBug, tp_key, Qnil);
     }
 
     if (RTEST(proc)) {
@@ -57,8 +58,9 @@ set_gc_hook(VALUE module, VALUE proc, rb_event_flag_t event, const char *tp_str,
             rb_raise(rb_eTypeError, "trace_func needs to be Proc");
         }
 
-        tpval = rb_tracepoint_new(0, event, gc_start_end_i, (void *)proc);
-        rb_ivar_set(module, tp_key, tpval);
+        rb_ivar_set(tp_mBug, rb_intern(proc_str), proc);
+        tpval = rb_tracepoint_new(0, event, gc_start_end_i, (void *)proc_str);
+        rb_ivar_set(tp_mBug, tp_key, tpval);
         rb_tracepoint_enable(tpval);
     }
 
@@ -66,16 +68,16 @@ set_gc_hook(VALUE module, VALUE proc, rb_event_flag_t event, const char *tp_str,
 }
 
 static VALUE
-set_after_gc_start(VALUE module, VALUE proc)
+set_after_gc_start(VALUE _self, VALUE proc)
 {
-    return set_gc_hook(module, proc, RUBY_INTERNAL_EVENT_GC_START,
+    return set_gc_hook(proc, RUBY_INTERNAL_EVENT_GC_START,
                        "__set_after_gc_start_tpval__", "__set_after_gc_start_proc__");
 }
 
 static VALUE
-start_after_gc_exit(VALUE module, VALUE proc)
+start_after_gc_exit(VALUE _self, VALUE proc)
 {
-    return set_gc_hook(module, proc, RUBY_INTERNAL_EVENT_GC_EXIT,
+    return set_gc_hook(proc, RUBY_INTERNAL_EVENT_GC_EXIT,
                        "__set_after_gc_exit_tpval__", "__set_after_gc_exit_proc__");
 }
 
diff --git a/ext/-test-/tracepoint/tracepoint.c b/ext/-test-/tracepoint/tracepoint.c
index 2826cc038c..001d9513b2 100644
--- a/ext/-test-/tracepoint/tracepoint.c
+++ b/ext/-test-/tracepoint/tracepoint.c
@@ -1,6 +1,8 @@
 #include "ruby/ruby.h"
 #include "ruby/debug.h"
 
+VALUE tp_mBug;
+
 struct tracepoint_track {
     size_t newobj_count;
     size_t free_count;
@@ -89,8 +91,8 @@ void Init_gc_hook(VALUE);
 void
 Init_tracepoint(void)
 {
-    VALUE mBug = rb_define_module("Bug");
-    Init_gc_hook(mBug);
-    rb_define_module_function(mBug, "tracepoint_track_objspace_events", tracepoint_track_objspace_events, 0);
-    rb_define_module_function(mBug, "tracepoint_specify_normal_and_internal_events", tracepoint_specify_normal_and_internal_events, 0);
+    tp_mBug = rb_define_module("Bug"); // GC root
+    Init_gc_hook(tp_mBug);
+    rb_define_module_function(tp_mBug, "tracepoint_track_objspace_events", tracepoint_track_objspace_events, 0);
+    rb_define_module_function(tp_mBug, "tracepoint_specify_normal_and_internal_events", tracepoint_specify_normal_and_internal_events, 0);
 }
diff --git a/test/-ext-/tracepoint/test_tracepoint.rb b/test/-ext-/tracepoint/test_tracepoint.rb
index bf66d8f105..debddd83d0 100644
--- a/test/-ext-/tracepoint/test_tracepoint.rb
+++ b/test/-ext-/tracepoint/test_tracepoint.rb
@@ -83,7 +83,7 @@ class TestTracepointObj < Test::Unit::TestCase
   end
 
   def test_teardown_with_active_GC_end_hook
-    assert_separately([], 'require("-test-/tracepoint"); Bug.after_gc_exit_hook = proc {}')
+    assert_separately([], 'require("-test-/tracepoint"); Bug.after_gc_exit_hook = proc {}; GC.start')
   end
 
 end