Skip to content

Commit d8c059e

Browse files
authored
Add a spec for the GC window inside the first #field_types call (#1456)
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 #1454.
1 parent 401b999 commit d8c059e

1 file changed

Lines changed: 25 additions & 0 deletions

File tree

spec/mysql2/result_spec.rb

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,31 @@
9595
expect(result.field_types).to eql(before_types)
9696
end
9797

98+
it "should keep field_types valid when GC runs during the first call" do
99+
# The array is stored on the C struct before the fill loop runs, and that
100+
# loop allocates a String per column, so a GC inside it can collect the
101+
# still-unmarked array between the store and the rb_ary_store that follows.
102+
# That window is a write into a freed slot, and it is inside the very first
103+
# #field_types call -- the spec above only covers a stale read on a later
104+
# one. Ordinary GC is enough here; compaction is not part of the mechanism.
105+
# Reproduction from @jeremy (#1453). Three fresh results give three
106+
# independent first-call windows: whether the dying array temporary is
107+
# conservatively pinned on the C stack is compiler/layout luck, so one
108+
# window could theoretically survive unpatched where another aborts.
109+
3.times do
110+
result = @client.query "SELECT 1 AS a, 'x' AS b, 2.5 AS c, NOW() AS d"
111+
begin
112+
GC.stress = true
113+
types = result.field_types # first-ever call on this Result
114+
ensure
115+
GC.stress = false
116+
end
117+
expect(types).to be_an_instance_of(Array)
118+
expect(types.length).to eql(4)
119+
expect(types).to all(be_an_instance_of(String))
120+
end
121+
end
122+
98123
it "should raise a Mysql2::Error exception upon a bad query" do
99124
expect do
100125
@client.query "bad sql"

0 commit comments

Comments
 (0)