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.
This commit is contained in:
Alan Wu 2025-09-02 17:42:49 -04:00
parent 4c0b68156d
commit 61d5fb213d
3 changed files with 21 additions and 17 deletions

View File

@ -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__");
}

View File

@ -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);
}

View File

@ -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