Skip to content

gh-109564: check server._sockets before attaching a new transport - #154861

Open
zero-context wants to merge 1 commit into
python:mainfrom
zero-context:fix-gh-109564
Open

gh-109564: check server._sockets before attaching a new transport#154861
zero-context wants to merge 1 commit into
python:mainfrom
zero-context:fix-gh-109564

Conversation

@zero-context

@zero-context zero-context commented Jul 29, 2026

Copy link
Copy Markdown

Independent, narrower alternative to #131009 (the _ServerState enum
redesign) — 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() 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, _attach()'s assert self._sockets is not None fires.

Reproduced deterministically with real sockets (no mocking): closing
the server after exactly 3 asyncio.sleep(0) yields following a client
connect crashes 300/300 times on current main; every other yield
count, 0/300. The same trigger also produces a second, previously
undocumented crash: a TypeError: 'NoneType' object is not iterable in
_wakeup(), fired during garbage collection when the never-fully-
attached transport's __del__ calls _detach() a second time. That one
doesn't show up in loop.call_exception_handler() output at all —
__del__ failures are reported through a separate path — so it's easy
to 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.

--- a/Lib/asyncio/selector_events.py
+++ b/Lib/asyncio/selector_events.py
@@ -218,6 +218,16 @@
         protocol = None
         transport = None
         try:
+            # gh-109564: create_task() defers this coroutine's first
+            # step by at least one loop iteration, so the server can
+            # legitimately close() in the gap between "connection
+            # accepted" and "this task actually runs". Attaching to an
+            # already-closed server crashes deep inside transport
+            # construction (Server._attach()'s assert); check up front
+            # instead and drop the already-accepted connection cleanly.
+            if server is not None and server._sockets is None:
+                conn.close()
+                return
             protocol = protocol_factory()
             waiter = self.create_future()
             if sslcontext:

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.d entry.

Validated on current main: the new tests fail without the fix and
pass with it; the full test_asyncio suite passes (2,811 tests, 0
failures, same skip set before and after); Tools/patchcheck/patchcheck.py
runs clean.

Why this doesn't implement the #131009 design, and one thing it doesn't cover

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.
@python-cla-bot

python-cla-bot Bot commented Jul 29, 2026

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant