Skip to content

Possible memory corruption fix#40

Open
benubois wants to merge 3 commits into
bryanp:mainfrom
benubois:memory_corruption
Open

Possible memory corruption fix#40
benubois wants to merge 3 commits into
bryanp:mainfrom
benubois:memory_corruption

Conversation

@benubois

Copy link
Copy Markdown

Hi,

Since upgrading to http ~> 6, I've seen a new sporadic exception.

The backtrace ends at an http file, but I think it's at a point where it reading data from llhttp:

TypeError: wrong argument type Hash (expected Class) parser << data ^^^^

[GEM_ROOT]/gems/http-6.0.4/lib/http/response/parser.rb#57 in HTTP::Response::Parser#add
[GEM_ROOT]/gems/http-6.0.4/lib/http/connection/internals.rb#134 in HTTP::Connection::Internals#read_more

I have zero understanding of the C extension, so sorry if this is slop, but Claude came up with the below solution and explanation.

Here's the value I was able to add on top of the LLM:

  1. Tests pass in llhttp
  2. Tests pass with this fix in http
  3. I'm running the patched llhttp in production in a Sidekiq cluster which downloads around 600 urls per second and haven't seen this exception come back

Thank you!

Warning

LLM output below

LLHttp::Parser caches the delegate in its C struct: parserData->delegate = rb_iv_get(self, "@delegate"); but the data type registered no GC mark callback ({0, rb_llhttp_free, rb_llhttp_memsize}). The @delegate ivar keeps the object alive, yet under GC compaction the delegate is relocated: the ivar reference is updated, while the cached copy in the struct is not. The next callback then dispatches against a stale/reused slot, producing a hard interpreter crash or a type-confusion error such as TypeError: wrong argument type Hash (expected Class) — even for perfectly valid input.

This is timing-dependent (compaction must run between parser creation and use), which is why it surfaces intermittently in high-throughput, GC-heavy workloads rather than in simple scripts.

Fix

Add a mark function that marks the cached delegate and register it for both the TypedData and legacy Data_Make_Struct paths. rb_gc_mark also pins the delegate, which is what's needed since a bare VALUE cached outside the Ruby heap can't tolerate being moved. Works across the whole supported Ruby range (>= 2.6.7) with no version guards.

Test

spec/integration/compaction_spec.rb builds a parser whose delegate is reachable only through the ivar, forces relocation with GC.verify_compaction_references(expand_heap: true, toward: :empty), then parses. It runs in a child process because the pre-fix failure can be an uncatchable crash. The test fails (child crashes) without the fix and passes with it.

@benubois
benubois force-pushed the memory_corruption branch from 456ed6d to dc98d7b Compare July 19, 2026 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant