gh-109564: check server._sockets before attaching a new transport - #154861
Open
zero-context wants to merge 1 commit into
Open
gh-109564: check server._sockets before attaching a new transport#154861zero-context wants to merge 1 commit into
zero-context wants to merge 1 commit into
Conversation
Server.close() sets self._sockets = None synchronously, but _accept_connection2() runs as a Task, which always defers its first step by at least one event-loop iteration. If close() lands in that gap, Server._attach()'s assert self._sockets is not None fires. Check server._sockets is None before building the protocol/transport, and drop the already-accepted connection instead of attaching to a closed server.
zero-context
requested review from
1st1,
asvetlov,
kumaraditya303 and
willingc
as code owners
July 29, 2026 08:46
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.
Independent, narrower alternative to #131009 (the
_ServerStateenumredesign) — flagging that up front since #131009 is already open and
maintainer-preferred for this issue. This PR is a small, local guard
that fixes the specific reproduced crash; it doesn't replace the
broader design #131009 is doing.
Server.close()setsself._sockets = Nonesynchronously, but_accept_connection2()runs as aTask, which always defers its firststep by at least one event-loop iteration. If
close()lands in thatgap,
_attach()'sassert self._sockets is not Nonefires.Reproduced deterministically with real sockets (no mocking): closing
the server after exactly 3
asyncio.sleep(0)yields following a clientconnect crashes 300/300 times on current
main; every other yieldcount, 0/300. The same trigger also produces a second, previously
undocumented crash: a
TypeError: 'NoneType' object is not iterablein_wakeup(), fired during garbage collection when the never-fully-attached transport's
__del__calls_detach()a second time. That onedoesn't show up in
loop.call_exception_handler()output at all —__del__failures are reported through a separate path — so it's easyto miss even while actively looking at this bug. Both crashes are fixed
by the same change, confirmed 300/300 → 0/300 for the first and 300 → 0
occurrences for the second.
Also includes two new regression tests in
test_selector_events.py(deterministic, mock-based — no real-socket timing dependency, so
they're portable to CI) and a
Misc/NEWS.dentry.Validated on current
main: the new tests fail without the fix andpass with it; the full
test_asynciosuite passes (2,811 tests, 0failures, same skip set before and after);
Tools/patchcheck/patchcheck.pyruns clean.
Why this doesn't implement the #131009 design, and one thing it doesn't cover
Nonecheck forasyncio.Server._wakeup#126660 (closed,unmerged, targeted the
_wakeup()double-call symptom specifically)and GH-109564: add asyncio.Server state machine #131009 (open, the maintainer-preferred
_ServerStateenum,still awaiting core review). This PR is a narrower, local guard, not
a replacement for GH-109564: add asyncio.Server state machine #131009's broader design.
connection_made()firing asynchronously after
close()has already returned — isn'taddressed here.