Possible memory corruption fix

mri/ext/llhttp/llhttp_ext.c

@@ -28,6 +28,20 @@ typedef struct {
   ID on_header_value_complete;
 } rb_llhttp_parser_data;
 
+// Mark the cached delegate so GC keeps it alive. `rb_gc_mark` also pins the
+// object, which is required here: the delegate `VALUE` is cached in the C
+// struct, so if compaction moved it the cached copy would dangle.
+static void rb_llhttp_mark(void *data) {
+  llhttp_t *parser = data;
+  rb_llhttp_parser_data *parserData = (rb_llhttp_parser_data*) parser->data;
+
+  // `parserData` is `0` between allocation and initialization.
+  //
+  if (parserData != 0) {
+    rb_gc_mark(parserData->delegate);
+  }
+}
+
 static void rb_llhttp_free(void *data) {
   llhttp_t *parser = data;
   rb_llhttp_parser_data *parserData = (rb_llhttp_parser_data*) parser->data;
@@ -49,7 +63,7 @@ static size_t rb_llhttp_memsize(const void *data) {
 
 static const rb_data_type_t rb_llhttp_type = {
   "LLHttp::Parser",
-  {0, rb_llhttp_free, rb_llhttp_memsize,},
+  {rb_llhttp_mark, rb_llhttp_free, rb_llhttp_memsize,},
   0, 0, 0,
 };
 #endif
@@ -71,7 +85,7 @@ VALUE rb_llhttp_allocate(VALUE klass) {
 #ifdef TypedData_Make_Struct
   VALUE self = TypedData_Make_Struct(klass, llhttp_t, &rb_llhttp_type, parser);
 #else
-  VALUE self = Data_Make_Struct(klass, llhttp_t, 0, rb_llhttp_free, parser);
+  VALUE self = Data_Make_Struct(klass, llhttp_t, rb_llhttp_mark, rb_llhttp_free, parser);
 #endif
 
   // Set data to false so we know when the parser has been initialized.

spec/integration/compaction_spec.rb

@@ -0,0 +1,53 @@
+# frozen_string_literal: true
+
+# Regression test for a GC-compaction bug: the C parser struct caches the
+# delegate `VALUE` but registered no GC mark callback, so compaction could move
+# the delegate and leave the cached copy dangling -- crashing (or raising a
+# type-confusion error) the next time a callback fired.
+#
+# The failure can be a hard interpreter crash, so it runs in a child process.
+RSpec.describe "parsing after GC compaction" do
+  it "keeps the cached delegate valid across compaction" do
+    skip "compaction unavailable" unless GC.respond_to?(:verify_compaction_references)
+
+    script = <<~'RUBY'
+      require "llhttp"
+
+      class Delegate < LLHttp::Delegate
+        def on_header_field(_) = nil
+      end
+
+      # Build in a separate frame so the delegate is reachable only through the
+      # parser's @delegate ivar -- a movable heap slot compaction will relocate.
+      def build = LLHttp::Parser.new(Delegate.new, type: :response)
+
+      # Relocate every movable object to fresh pages. The heap-expansion keyword
+      # was named double_heap: through Ruby 3.1 and renamed expand_heap: in 3.2.
+      def relocate
+        GC.verify_compaction_references(expand_heap: true, toward: :empty)
+      rescue ArgumentError
+        begin
+          GC.verify_compaction_references(double_heap: true, toward: :empty)
+        rescue ArgumentError
+          GC.verify_compaction_references(toward: :empty)
+        end
+      end
+
+      10.times do
+        parser = build
+        relocate
+        parser << "HTTP/1.1 200 OK\r\nX-Test: yes\r\n\r\n"
+      end
+
+      print "COMPACTION_SURVIVED"
+    RUBY
+
+    # stderr is merged in so a crash backtrace shows in the failure message;
+    # match a sentinel rather than the whole stream, which may carry unrelated
+    # RubyGems warnings from the child's environment.
+    output = IO.popen([RbConfig.ruby, *$LOAD_PATH.flat_map { |p| ["-I", p] }, "-e", script], err: %i[child out], &:read)
+
+    expect($?).to be_success, "child process crashed:\n#{output}"
+    expect(output).to include("COMPACTION_SURVIVED")
+  end
+end