Fix use-after-free crash when close races with ping or query

[bquorning]

Summary

Fix a use-after-free crash when close is called from one thread while another thread is executing ping, query, or another operation without the GVL. Instead of calling mysql_close immediately (which frees the MYSQL struct while another thread is still using it), close now detects whether an operation is in-flight and takes one of two paths:

• Idle (no in-flight operation): call mysql_close directly while holding the GVL for a clean COM_QUIT disconnect.
• Busy (operation in-flight): set a closed flag and call shutdown(SHUT_RDWR) on the socket to interrupt the blocked I/O, then defer mysql_close to decr_mysql2_client (refcount == 0) where no concurrent access is possible.

Reproducer:

client = Mysql2::Client.new(host: "127.0.0.1", port: 3306, username: "root")
Thread.new { client.ping }
client.close

This crashes with a segfault because both nogvl_close and nogvl_ping release Ruby's GVL via rb_thread_call_without_gvl and then call libmysql functions concurrently on the same MYSQL * connection. mysql_close frees internal state while mysql_ping is still accessing it.

Why not a native mutex

A native mutex was considered but intentionally avoided: if a thread holds such a mutex at fork() time, the child inherits a permanently-locked mutex and any subsequent close or GC deadlocks. The flag + shutdown() approach avoids this problem entirely.

A Ruby-level Mutex was also discussed and dismissed in #1284 by @casperisfine, with valid arguments about method dispatch overhead, Thread.handle_interrupt handling, and behavioral changes.

Why the connection pool cannot prevent this

The pool serializes starting operations at the Ruby level, but once two threads have each passed the Ruby-level checks (one doing ping, the other doing close), they are inside rb_thread_call_without_gvl concurrently. The pool has no visibility into that window.

Changes

• Split close into idle/busy paths as described above
• Set active_fiber around ping's nogvl call so close can detect in-flight pings
• Check wrapper->closed in REQUIRE_CONNECTED, closed?, and ping since mysql_close is now deferred and CONNECTED() remains true after close
• Add a doc comment to client.c explaining the three thread safety mechanisms (active_fiber, closed flag + shutdown, refcount)
• Add tests for concurrent close+ping, close+query, and fork safety

Related issues

• mysql2 doesn't seem to be threadsafe #461 — "mysql2 doesn't seem to be threadsafe" — shows the exact crash pattern.
• mysql_ping causing Thread interrupts in Phusion passenger #777 — mysql_ping crash with thread contention in Passenger.
• Unknown MySQL error: BEGIN, undefined method `rows' #983, mysql2 gem crashes randomly #1019, Segmentation faults when running GraphQL tests (Rails, Minitest) #1282 — segfault reports likely caused by the same family of races.

Test plan

• Added specs for concurrent ping+close, query+close, and fork safety
• Existing test suite passes
• rake spec:valgrind passes

🤖 Generated with Claude Code

[HeyNonster]

I think if we do client.query("SELECT SLEEP(1)") then we can get it to consistently fail without worrying about the flakiness of ping executing before client.close does. That way we can probably remove the 100.times do.

[HeyNonster]

That said, the block runs 100 times slightly faster than 1 second so SELECT SLEEP(1) would be moderately slower.

[sodabrew]

This is great, thank you! Kicking off the CI run

[byroot]

Currently at a conference so can't properly review the PR before a few days, but just based on the description I'd advise against using a native mutex, as it if the thread owning it dies (typically on fork) it could result in a deadlock of the VM.

I suspect there is a simpler fix for this race.

[byroot]

So I got a few minutes to look, and yes, I believe the proper fix is to extend the use of rb_mysql_client_set_active_fiber to ping and close (and perhaps others?).

[bquorning]

I’ll try that @byroot. Thanks for the review.

[bquorning]

FYI @sodabrew CI seems to be failing because of expired signatures. I am not sure how to update them, but I suspect CI will continue to fail until that has been fixed.

[bquorning]

This spec may be too heavy-handed to keep around. Let me know if I should delete it before merge.

[bquorning]

I believe the proper fix is to extend the use of rb_mysql_client_set_active_fiber to ping and close (and perhaps others?).

I am not very comfortable in C and relying a lot on Claude Code (and you) to make the right decisions here 😅 What I’m getting back is that

• in ping we want to return true or false, but calling rb_mysql_client_set_active_fiber will raise an exception if the connection is already in use.
• And in the close method we don’t want to claim the connection. We want to check if anybody else is using it.

Instead we are adding a NIL_P(wrapper->active_fiber) check, which is identical to other checks already in the code.

[byroot]

• n ping we want to return true or false, but calling rb_mysql_client_set_active_fiber will raise an exception if the connection is already in use.

I believe it's fine, it's already the design of mysql2 that concurrent usage raises. This should never happen, if it does the caller is doing something wrong.

And in the close method we don’t want to claim the connection. We want to check if anybody else is using it.

Yes, but we also don't want anyone starting using it while we're closing it, which is achieved by setting the active fiber.

The active_fiber is effectively a cheaper mutex, that raise when contented.

After glancing at your PR, one problem remaining is that you must either ensure that the APIs you are calling between setting and unsetting the current fiber will never raise, or use rb_protect (or other API to handle exceptions)

[byroot]

I don't think this is necessary. We can rely on initialized = 0, Mysql2::Client can reconnect.

[byroot]

Simpler fix in #1436

[bquorning]

Simpler fix in #1436

That looks so much better than my approach. I’ll close this PR.

[bquorning]

FYI @sodabrew CI seems to be failing because of expired signatures. I am not sure how to update them, but I suspect CI will continue to fail until that has been fixed.

Oh, a fix for the failing signatures on CI was suggested in #1424 back in February.