diff --git a/Lib/asyncio/base_events.py b/Lib/asyncio/base_events.py index 5dbe4b28d236d3..3312c6d734efb5 100644 --- a/Lib/asyncio/base_events.py +++ b/Lib/asyncio/base_events.py @@ -304,6 +304,15 @@ def _detach(self, transport): def _wakeup(self): waiters = self._waiters + if waiters is None: + # gh109564: the wakeup method has two possible call-sites, through an + # explicit call to the server's close method, or indirectly after the last + # client disconnects and the corresponding transport detaches from the + # server. These two can be in a race-condition if the server closes between + # `BaseSelectorEventLoop._accept_connection` and + # `BaseSelectorEventLoop._accept_connection2`; in this scenario we must + # check the wakeup call hasn't already set the server waiters to None. + return self._waiters = None for waiter in waiters: if not waiter.done(): diff --git a/Lib/asyncio/selector_events.py b/Lib/asyncio/selector_events.py index f94bf10b4225e7..4ec3311fbeb6dd 100644 --- a/Lib/asyncio/selector_events.py +++ b/Lib/asyncio/selector_events.py @@ -243,8 +243,10 @@ async def _accept_connection2( # It's now up to the protocol to handle the connection. except (SystemExit, KeyboardInterrupt): + conn.close() raise except BaseException as exc: + conn.close() if self._debug: context = { 'message': diff --git a/Lib/test/test_asyncio/test_server.py b/Lib/test/test_asyncio/test_server.py index 60a40cc8349fed..35962bf9eb7d73 100644 --- a/Lib/test/test_asyncio/test_server.py +++ b/Lib/test/test_asyncio/test_server.py @@ -1,11 +1,14 @@ import asyncio +import gc import os import socket import time import threading import unittest +from test import support from test.support import socket_helper +from test.support import warnings_helper from test.test_asyncio import utils as test_utils from test.test_asyncio import functional as func_tests @@ -266,6 +269,49 @@ async def serve(rd, wr): await asyncio.sleep(0) self.assertTrue(task.done()) + async def test_close_race(self): + + srv = await asyncio.start_server(lambda *_: None, socket_helper.HOSTv4, 0) + srv_sock = srv.sockets[0] + addr = srv_sock.getsockname() + + # When the server is closed before a connection is handled but after the + # connection is accepted, then a race-condition exists between the handler + # transport and the server, both which will attempt to wakeup the server to set + # any server waiters. We can recreate race-condition by opening a connection and + # waiting for the server reader callback before closing the server + loop = asyncio.get_running_loop() + srv_reader, _ = loop._selector.get_key(srv_sock.fileno()).data + conn_task = asyncio.create_task(asyncio.open_connection(addr[0], addr[1])) + for _ in range(10): + await asyncio.sleep(0) + if srv_reader in loop._ready: + break + + # Ensure accepted connection task is scheduled by the server reader, but not + # completed, before closing the server. + await asyncio.sleep(0) + srv.close() + + # Complete the client connection to close the socket. Suppress errors in the + # handler transport due to failing to attach to closed server. + with support.captured_stderr(): + try: + _, wr = await conn_task + self.addCleanup(wr.close) + except OSError: + pass + + await srv.wait_closed() + + # Verify the handler transport does not raise an error due to multiple calls to + # the server wakeup. Suppress expected ResourceWarnings from the handler + # transport failing to attach to the closed server. + with warnings_helper.check_warnings(("unclosed transport", ResourceWarning)), \ + support.catch_unraisable_exception() as cm: + support.gc_collect() + self.assertIsNone(cm.unraisable) + # Test the various corner cases of Unix server socket removal class UnixServerCleanupTests(unittest.IsolatedAsyncioTestCase): diff --git a/Misc/NEWS.d/next/Library/2024-11-11-11-13-30.gh-issue-109564._f7W0v.rst b/Misc/NEWS.d/next/Library/2024-11-11-11-13-30.gh-issue-109564._f7W0v.rst new file mode 100644 index 00000000000000..70e981a8a5dd5f --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-11-11-11-13-30.gh-issue-109564._f7W0v.rst @@ -0,0 +1 @@ +Fix race condition in :meth:`asyncio.Server.close`. Patch by Jamie Phan.