Repository navigation
Fix call_soon_threadsafe() racing with close() on free-threaded Python - #789
Open
maartenbreddels wants to merge 1 commit into
Open
maartenbreddels wants to merge 1 commit into
maartenbreddels wants to merge 1 commit into
Conversation
call_soon_threadsafe() checks _closed, appends to _ready and then calls handler_async.send(). On a free-threaded build another thread can run close() in between: it closes handler_async and sets it to None, so the poster segfaults in send(). The same race also makes close() fail with "deque mutated during iteration" or "new callbacks were queued during loop closing". With the GIL the method runs as one step, so this did not happen. A critical section on the loop is not enough: the deque append can block on the deque's own lock, and that suspends the outer critical section. A PyMutex stays held while the thread blocks. The mutex is not reentrant, so nothing that can start a GC runs under it: a finalizer could post to the same loop and deadlock. That is why the closed-loop error is raised after the lock is released. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
maartenbreddels
marked this pull request as ready for review
November 8, 2026 12:04
Vizonex
approved these changes
Nov 8, 2026
Vizonex
left a comment
There was a problem hiding this comment.
This makes sense to me as to why this would be in place. I'll have to think about implementing this on winloop's end if needed. There seems to be a workflow broken but it's not the library's fault so I approve of this PR.
Author
|
Thanks for the review! The red job is We also tested the fix on Linux, in our app's 3.14t image:
|
|
@maartenbreddels In the meantime I've added this bugfix on winloop's side Vizonex/Winloop#189 |
3 tasks done
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.
Fixes #788.
On a free-threaded build,
call_soon_threadsafe()can run while another thread closes the loop.The poster can pass the
_closedcheck, and then_close()setshandler_asynctoNone.The poster then calls
send()onNoneand the process segfaults.The same race also makes
close()fail withdeque mutated during iterationornew callbacks were queued during loop closing.With the GIL, the method runs as one step, so this did not happen.
This is a different race than #720.
The fix
The loop gets a
cython.pymutex(_close_lock).call_soon_threadsafe()holds it while it reads_closed, appends to_ready, and callshandler_async.send()._close()holds it only while it checks and sets_closed.After that, no poster touches
_readyorhandler_asyncagain, so the rest of_close()runs without the lock.The lock is not reentrant, so no code that can allocate Python objects runs under it.
An allocation can start a GC, and a finalizer can call
call_soon_threadsafe()on the same loop.For that reason the poster raises
RuntimeError('Event loop is closed')only after it releases the lock.For the same reason,
_close()cancels the handles in_readyafter it releases the lock.I first tried
with cython.critical_section(self).It fixed the segfault, but
close()still failed 1 to 3 times in 20 seconds.The reason: the deque append can block on the deque's own lock, and a blocked thread suspends its critical sections.
The closing thread can then run while the poster is between the check and the append.
A
PyMutexstays held while the thread blocks, so it does not have this gap.call_soon_threadsafe()is the only way other threads post callbacks to the loop.Other threads may also call
is_closed(), which only reads a flag.Signal handling runs on the loop thread.
The async generator finalizer hook can run on any thread, but it also posts through
call_soon_threadsafe().shutdown_default_executor()andset_debug()post throughcall_soon_threadsafe(), so they get the fix too.Tests
New test:
test_call_soon_threadsafe_while_closing(UV and AIO).Two threads create, run, and close loops while four threads post to them, for 1 second.
The only error a poster may get is
RuntimeError('Event loop is closed'); any other error fails the test.The threads are daemon threads and the join has a timeout, so a deadlock fails the test instead of hanging the suite.
On
masterit fails 3 of 3 runs on 3.14t (close()errors).With the fix it passes.
I ran these tests on 3.14t, 3.14, and 3.9, all pass:
test_call_soon_threadsafe_while_closing,test_call_soon_threadsafe_safety,test_freethreading,test_close, forTestBaseUVandTestBaseAIO.test_freethreadingskips on the GIL builds, as expected.The standalone repro from the issue, on 3.14t: three 20-second runs and one 60-second run, 0 crashes and 0
close()failures.The 60-second run made 118 million posts.
Before the fix it crashes in under a second.
A GC re-entry check (a
__del__on a cycle that posts to a closed loop, with a low GC threshold) does not deadlock on 3.9, 3.14, and 3.14t.Cost, on an M-series Mac:
call_soon_threadsafe(), with and without the fix, on 3.14 and 3.14t.Disclosure: the analysis, the fix, and the tests were written with Claude Code, and reviewed by three AI reviewers. The crash comes from our own production-like load tests.