diff --git a/ext/fiddle/closure.c b/ext/fiddle/closure.c index 5d936473..45713ffd 100644 --- a/ext/fiddle/closure.c +++ b/ext/fiddle/closure.c @@ -7,6 +7,7 @@ int ruby_thread_has_gvl_p(void); /* from internal.h */ VALUE cFiddleClosure; typedef struct { + VALUE self; void * code; ffi_closure *pcl; ffi_cif cif; @@ -54,12 +55,35 @@ closure_memsize(const void * ptr) return size; } +/* Fiddle::Closure#free and initialize_rescue set the data pointer to NULL. + * The garbage collector does not call these functions with a NULL pointer, + * but this file is the only one in the extension that clears the pointer, + * so both functions check it. */ +static void +closure_mark(void *ptr) +{ + fiddle_closure *closure = ptr; + if (closure) { + rb_gc_mark_movable(closure->self); + } +} + +static void +closure_compact(void *ptr) +{ + fiddle_closure *closure = ptr; + if (closure) { + closure->self = rb_gc_location(closure->self); + } +} + const rb_data_type_t closure_data_type = { .wrap_struct_name = "fiddle/closure", .function = { - .dmark = 0, + .dmark = closure_mark, .dfree = dealloc, - .dsize = closure_memsize + .dsize = closure_memsize, + .dcompact = closure_compact }, .flags = FIDDLE_DEFAULT_TYPED_DATA_FLAGS, }; @@ -76,7 +100,7 @@ with_gvl_callback(void *ptr) { struct callback_args *x = ptr; - VALUE self = (VALUE)x->ctx; + VALUE self = ((fiddle_closure *)x->ctx)->self; VALUE rbargs = rb_iv_get(self, "@args"); VALUE ctype = rb_iv_get(self, "@ctype"); int argc = RARRAY_LENINT(rbargs); @@ -288,6 +312,10 @@ initialize_body(VALUE user_data) TypedData_Get_Struct(data->self, fiddle_closure, &closure_data_type, cl); + /* libffi receives the address of this struct, which xmalloc'd memory keeps + * stable. The trampoline reads cl->self, so the GC must mark and relocate it. */ + RB_OBJ_WRITE(data->self, &cl->self, data->self); + cl->argv = (ffi_type **)xcalloc(argc + 1, sizeof(ffi_type *)); normalized_args = rb_ary_new_capa(argc); @@ -318,9 +346,9 @@ initialize_body(VALUE user_data) #if USE_FFI_CLOSURE_ALLOC result = ffi_prep_closure_loc(pcl, cif, callback, - (void *)(data->self), cl->code); + (void *)cl, cl->code); #else - result = ffi_prep_closure(pcl, cif, callback, (void *)(data->self)); + result = ffi_prep_closure(pcl, cif, callback, (void *)cl); cl->code = (void *)pcl; i = mprotect(pcl, sizeof(*pcl), PROT_READ | PROT_EXEC); if (i) { diff --git a/test/fiddle/test_closure.rb b/test/fiddle/test_closure.rb index ee9ef044..59a92015 100644 --- a/test/fiddle/test_closure.rb +++ b/test/fiddle/test_closure.rb @@ -172,6 +172,31 @@ def call end end + def test_call_after_compaction + unless GC.respond_to?(:verify_compaction_references) + omit("Need GC.verify_compaction_references") + end + omit("Need CRuby") unless RUBY_ENGINE == "ruby" + + closure_class = Class.new(Closure) do + def call(a, b) + a + b + end + end + + # The closure must be reachable only through the heap. A local variable is + # pinned by the conservative machine-stack scan. A pinned closure does not + # move, and then this test cannot fail. + holder = [closure_class.new(TYPE_INT, [TYPE_INT, TYPE_INT])] + begin + GC.verify_compaction_references(expand_heap: true, toward: :empty) + func = Function.new(holder[0].to_i, [TYPE_INT, TYPE_INT], TYPE_INT) + assert_equal(42, func.call(40, 2)) + ensure + holder[0].free + end + end + def test_ractor_shareable omit("Need Ractor") unless defined?(Ractor) closure_class = Class.new(Closure) do