Stabilize detach race spec by removing cross-thread detach#100
Merged
Conversation
The first example in detach_race_condition_spec.rb detached a watcher from a separate thread while the loop was running run_once. During the poll the GVL is released, so that detach called ev_io_stop concurrently with the loop's backend_poll on the same ev_loop. libev is not thread-safe, so this corrupted its internal watcher lists and crashed intermittently with a SEGV on macOS (kqueue), reliably reproducible under load. Rewrite the example to reproduce issue socketry#87 deterministically within a single thread: several watchers have an event pending in the same loop iteration, and the first one dispatched detaches the others. The loop must skip their stale pending events rather than dispatch them to a detached watcher. This exercises the same dispatch/skip path (verified: breaking it reproduces the original "TypeError: wrong argument type nil") without the unsupported concurrent mutation of libev, and matches how the library is actually used (single loop per thread). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
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.
Problem
The first example in
spec/detach_race_condition_spec.rbdetached a watcher from a separate thread while the loop was runningrun_once. During the poll the GVL is released (libev is patched to do so), so thatdetachended up callingev_io_stopconcurrently with the loop'sbackend_pollon the sameev_loop.libev is not thread-safe, so this corrupted its internal watcher lists and crashed intermittently with a SEGV on macOS (kqueue) — reliably reproducible under load:
The fault is inside
ev_io_stop()(wlist_del/fd_changewalkinganfds[fd]) racing against the loop thread's post-keventfd_eventprocessing.Fix
Rewrite the example to reproduce issue #87 deterministically within a single thread: several watchers have an event pending in the same loop iteration, and the first one dispatched detaches the others. The loop must skip their now-stale pending events rather than dispatch them to a detached watcher.
This exercises the same dispatch/skip path — verified: temporarily breaking the skip logic reproduces the original
TypeError: wrong argument type nil (expected Coolio::Loop)— but without the unsupported concurrent mutation of libev, and it matches how the library is actually used (a single loop per thread).Notes / trade-off
The cross-thread
detach-during-run_oncescenario is genuinely unsupported: libev is not thread-safe and cool.io releases the GVL around the poll syscall, so mutating a running loop from another thread cannot be made safe without redesigning the loop's locking. That path is therefore no longer exercised by this spec.Verification
49 examples, 0 failures🤖 Generated with Claude Code