Keep the Closure in place for libffi across GC compaction

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) {

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