Skip to content

Commit 489b225

Browse files
committed
gh-109564: Check server._sockets before attaching a new transport
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.
1 parent 9ccd5bb commit 489b225

3 files changed

Lines changed: 53 additions & 0 deletions

File tree

Lib/asyncio/selector_events.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,16 @@ async def _accept_connection2(
225225
protocol = None
226226
transport = None
227227
try:
228+
# gh-109564: create_task() defers this coroutine's first
229+
# step by at least one loop iteration, so the server can
230+
# legitimately close() in the gap between "connection
231+
# accepted" and "this task actually runs". Attaching to an
232+
# already-closed server crashes deep inside transport
233+
# construction (Server._attach()'s assert); check up front
234+
# instead and drop the already-accepted connection cleanly.
235+
if server is not None and server._sockets is None:
236+
conn.close()
237+
return
228238
protocol = protocol_factory()
229239
waiter = self.create_future()
230240
if sslcontext:

Lib/test/test_asyncio/test_selector_events.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,45 @@ def test_accept_connection_reschedules_once_on_resource_error(self):
421421
self.assertEqual(self.loop.call_exception_handler.call_count, 1)
422422
self.assertEqual(self.loop.call_later.call_count, 1)
423423

424+
def test_accept_connection2_server_already_closed(self):
425+
# gh-109564: Server.close() sets Server._sockets = None
426+
# synchronously, but _accept_connection2() only starts running at
427+
# least one event-loop iteration after it's scheduled (it's a
428+
# Task). If close() lands in that gap, attaching a transport to
429+
# the now-closed server used to raise an uncatchable
430+
# AssertionError deep inside transport construction instead of
431+
# just dropping the already-accepted connection.
432+
conn = mock.Mock()
433+
protocol_factory = mock.Mock()
434+
server = mock.Mock()
435+
server._sockets = None
436+
437+
coro = self.loop._accept_connection2(
438+
protocol_factory, conn, {}, server=server)
439+
self.loop.run_until_complete(coro)
440+
441+
conn.close.assert_called_with()
442+
protocol_factory.assert_not_called()
443+
444+
def test_accept_connection2_no_server(self):
445+
# _accept_connection2's server parameter defaults to None (no
446+
# Server object to check), which must not itself raise -- only
447+
# an actual closed Server should short-circuit.
448+
conn = mock.Mock()
449+
protocol = mock.Mock()
450+
protocol_factory = mock.Mock(return_value=protocol)
451+
waiter = self.loop.create_future()
452+
waiter.set_result(None)
453+
self.loop.create_future = mock.Mock(return_value=waiter)
454+
self.loop._make_socket_transport = mock.Mock()
455+
456+
coro = self.loop._accept_connection2(
457+
protocol_factory, conn, {}, server=None)
458+
self.loop.run_until_complete(coro)
459+
460+
protocol_factory.assert_called_with()
461+
conn.close.assert_not_called()
462+
424463
class SelectorTransportTests(test_utils.TestCase):
425464

426465
def setUp(self):
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix a race in :class:`asyncio.Server` where calling :meth:`~asyncio.Server.close`
2+
while a connection was in the process of being accepted could raise an
3+
uncatchable :exc:`AssertionError` deep inside transport creation instead of
4+
simply dropping the connection.

0 commit comments

Comments
 (0)