summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlan Wu <XrXr@users.noreply.github.com>2025-09-02 17:42:49 -0400
committerAlan Wu <XrXr@users.noreply.github.com>2025-09-02 18:28:20 -0400
commit61d5fb213d48a329e9978dc0c5deb4c5f30c1068 (patch)
treed3554047ad06bae94e799ebe9df96c552d4aaeac
parent4c0b68156d35b7149dc4cbd9d402d495daf95776 (diff)
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.
-rw-r--r--ext/-test-/tracepoint/gc_hook.c26
-rw-r--r--ext/-test-/tracepoint/tracepoint.c10
-rw-r--r--test/-ext-/tracepoint/test_tracepoint.rb2
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