Add a regression spec for the GC window inside the first #field_types call - #1456
Merged
sodabrew merged 1 commit intoAug 6, 2026
Merged
Conversation
The existing spec calls #field_types to completion before collecting, so it covers a stale read on a later call. The window that actually goes wrong is inside the first call: the array is stored on the wrapper before the fill loop runs, and that loop allocates a String per column, so a GC there can collect the still-unmarked array between the store and the rb_ary_store that follows. Three fresh results give three independent first-call windows, since whether the dying array temporary is conservatively pinned on the C stack is compiler/layout luck; the contents assertion catches a corrupted-but-alive array as well as a crash. Unpatched, this aborts with "[BUG] try to mark T_NONE object" 3/3 under GC.stress; patched it is clean 3/3. Reproduction from @jeremy, who found the same bug independently in brianmario#1454.
sodabrew
added a commit
to sodabrew/mysql2
that referenced
this pull request
Aug 7, 2026
CI on this branch has been consistently hitting "[BUG] try to mark T_NONE object" at the exact line brianmario#1456's regression spec targets (result_spec.rb:113), across every OS/DB combination in both the Build and Container workflows -- 6/6 non-infra job failures on the last push, all the same crash. Upstream master's own recent CI failures are unrelated (a ruby-head C-API incompatibility, a couple of environment-specific flaky specs) and show zero instances of this crash, so this is specific to something on this branch, even though it was not reliably reproducible locally in a controlled comparison (confounded by concurrent background-agent activity on this machine while investigating). rb_mysql_result_fetch_fields and rb_mysql_result_fetch_field_types both follow the same shape: create an Array, store it directly on the C struct's wrapper->fields/wrapper->fieldTypes, then loop, allocating a Ruby String (a GC safepoint) per column and storing it into that array. Between the initial store and the loop finishing, the array is reachable only through the wrapper -- there is no live reference to it on the C stack. Under GC.stress a mark pass can land in that gap; per brianmario#1456's own description, whether the array happens to survive anyway is "compiler/layout luck". Capture a stack-local VALUE for each array before the loop and RB_GC_GUARD it after, so conservative stack scanning always finds the array too, independent of that timing. Same idiom already used elsewhere in this codebase for the same class of concern.
sodabrew
added a commit
to sodabrew/mysql2
that referenced
this pull request
Aug 8, 2026
CI on this branch has been consistently hitting "[BUG] try to mark T_NONE object" at the exact line brianmario#1456's regression spec targets (result_spec.rb:113), across every OS/DB combination in both the Build and Container workflows -- 6/6 non-infra job failures on the last push, all the same crash. Upstream master's own recent CI failures are unrelated (a ruby-head C-API incompatibility, a couple of environment-specific flaky specs) and show zero instances of this crash, so this is specific to something on this branch, even though it was not reliably reproducible locally in a controlled comparison (confounded by concurrent background-agent activity on this machine while investigating). rb_mysql_result_fetch_fields and rb_mysql_result_fetch_field_types both follow the same shape: create an Array, store it directly on the C struct's wrapper->fields/wrapper->fieldTypes, then loop, allocating a Ruby String (a GC safepoint) per column and storing it into that array. Between the initial store and the loop finishing, the array is reachable only through the wrapper -- there is no live reference to it on the C stack. Under GC.stress a mark pass can land in that gap; per brianmario#1456's own description, whether the array happens to survive anyway is "compiler/layout luck". Capture a stack-local VALUE for each array before the loop and RB_GC_GUARD it after, so conservative stack scanning always finds the array too, independent of that timing. Same idiom already used elsewhere in this codebase for the same class of concern.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #1453, which was merged while this spec was being added to it — the push and the merge crossed, so the spec landed on the branch a few hours after the branch had already gone in. The fix itself is in master; this PR carries only the regression spec.
It covers the window @jeremy described in #1453 (comment) (and in #1454):
rb_mysql_result_fetch_field_typesstores the new Array on the wrapper and then runs a fill loop that allocates a String per column, so a GC inside that loop could collect the still-unmarked array between the store and therb_ary_storethat follows — a write through a freed object slot, inside the very first#field_typescall. The pre-existing spec only covers a stale read on a later call. The test runs three fresh first-call windows underGC.stress, since whether the dying array temporary happens to be conservatively pinned on the C stack is compiler/layout luck, and asserts the contents as well as the shape.Verified it bites: with the two
fieldTypesmark/compact lines removed from current master, this spec aborts 3/3 with[BUG] try to mark T_NONE object; with master as-is it passes 3/3.Tested on Ruby 3.3.10, MySQL 9.6.0 (Homebrew, libmysqlclient.24), macOS arm64. The full suite there is 366 examples with 3 failures that are identical on unpatched current master (
client_spec.rb:101,:729,:740— the reconnect group). No dependency or code changes — one spec only.🤖 Generated with Claude Code