From dc98d7ba8ad97cc30f570170b3d699dc3dad647c Mon Sep 17 00:00:00 2001 From: Ben Ubois Date: Sun, 19 Jul 2026 00:47:03 +0200 Subject: [PATCH 1/3] Mark the delegate so the parser survives GC compaction --- mri/ext/llhttp/llhttp_ext.c | 18 +++++++++++-- spec/integration/compaction_spec.rb | 41 +++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 spec/integration/compaction_spec.rb diff --git a/mri/ext/llhttp/llhttp_ext.c b/mri/ext/llhttp/llhttp_ext.c index 92552fb..ee8fcc5 100644 --- a/mri/ext/llhttp/llhttp_ext.c +++ b/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. diff --git a/spec/integration/compaction_spec.rb b/spec/integration/compaction_spec.rb new file mode 100644 index 0000000..0931465 --- /dev/null +++ b/spec/integration/compaction_spec.rb @@ -0,0 +1,41 @@ +# 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) + + 10.times do + parser = build + GC.verify_compaction_references(expand_heap: true, toward: :empty) + 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 From fa602531f511f62a17a228a8f1462a953b6591c8 Mon Sep 17 00:00:00 2001 From: Ben Ubois Date: Sun, 19 Jul 2026 11:27:07 +0200 Subject: [PATCH 2/3] ci From 21f80ae30efef7ec5bec26c320fa655c33facde0 Mon Sep 17 00:00:00 2001 From: Ben Ubois Date: Sun, 19 Jul 2026 11:32:27 +0200 Subject: [PATCH 3/3] Test fix --- spec/integration/compaction_spec.rb | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/spec/integration/compaction_spec.rb b/spec/integration/compaction_spec.rb index 0931465..c1ca0dc 100644 --- a/spec/integration/compaction_spec.rb +++ b/spec/integration/compaction_spec.rb @@ -21,9 +21,21 @@ def on_header_field(_) = nil # 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 - GC.verify_compaction_references(expand_heap: true, toward: :empty) + relocate parser << "HTTP/1.1 200 OK\r\nX-Test: yes\r\n\r\n" end