Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-79033: Try to fix asyncio.Server.wait_closed() again #111336

Merged
merged 2 commits into from
Oct 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions Doc/library/asyncio-eventloop.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1619,8 +1619,9 @@ Do not instantiate the :class:`Server` class directly.
The sockets that represent existing incoming client connections
are left open.

The server is closed asynchronously, use the :meth:`wait_closed`
coroutine to wait until the server is closed.
The server is closed asynchronously; use the :meth:`wait_closed`
coroutine to wait until the server is closed (and no more
connections are active).

.. method:: get_loop()

Expand Down Expand Up @@ -1678,7 +1679,8 @@ Do not instantiate the :class:`Server` class directly.

.. coroutinemethod:: wait_closed()

Wait until the :meth:`close` method completes.
Wait until the :meth:`close` method completes and all active
connections have finished.

.. attribute:: sockets

Expand Down
24 changes: 22 additions & 2 deletions Lib/asyncio/base_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ def _wakeup(self):
self._waiters = None
for waiter in waiters:
if not waiter.done():
waiter.set_result(waiter)
waiter.set_result(None)

def _start_serving(self):
if self._serving:
Expand Down Expand Up @@ -377,7 +377,27 @@ async def serve_forever(self):
self._serving_forever_fut = None

async def wait_closed(self):
if self._waiters is None or self._active_count == 0:
"""Wait until server is closed and all connections are dropped.

- If the server is not closed, wait.
- If it is closed, but there are still active connections, wait.

Anyone waiting here will be unblocked once both conditions
willingc marked this conversation as resolved.
Show resolved Hide resolved
(server is closed and all connections have been dropped)
have become true, in either order.

Historical note: In 3.11 and before, this was broken, returning
immediately if the server was already closed, even if there
were still active connections. An attempted fix in 3.12.0 was
still broken, returning immediately if the server was still
open and there were no active connections. Hopefully in 3.12.1
we have it right.
"""
# Waiters are unblocked by self._wakeup(), which is called
# from two places: self.close() and self._detach(), but only
# when both conditions have become true. To signal that this
# has happened, self._wakeup() sets self._waiters to None.
if self._waiters is None:
return
waiter = self._loop.create_future()
self._waiters.append(waiter)
Expand Down
38 changes: 34 additions & 4 deletions Lib/test/test_asyncio/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,29 +122,59 @@ async def main(srv):

class TestServer2(unittest.IsolatedAsyncioTestCase):

async def test_wait_closed(self):
async def test_wait_closed_basic(self):
async def serve(*args):
pass

srv = await asyncio.start_server(serve, socket_helper.HOSTv4, 0)
self.addCleanup(srv.close)

# active count = 0
# active count = 0, not closed: should block
task1 = asyncio.create_task(srv.wait_closed())
await asyncio.sleep(0)
self.assertTrue(task1.done())
self.assertFalse(task1.done())

# active count != 0
# active count != 0, not closed: should block
srv._attach()
task2 = asyncio.create_task(srv.wait_closed())
await asyncio.sleep(0)
self.assertFalse(task1.done())
self.assertFalse(task2.done())

srv.close()
await asyncio.sleep(0)
# active count != 0, closed: should block
task3 = asyncio.create_task(srv.wait_closed())
await asyncio.sleep(0)
self.assertFalse(task1.done())
self.assertFalse(task2.done())
self.assertFalse(task3.done())

srv._detach()
# active count == 0, closed: should unblock
await task1
await task2
await task3
await srv.wait_closed() # Return immediately

async def test_wait_closed_race(self):
# Test a regression in 3.12.0, should be fixed in 3.12.1
async def serve(*args):
pass

srv = await asyncio.start_server(serve, socket_helper.HOSTv4, 0)
self.addCleanup(srv.close)

task = asyncio.create_task(srv.wait_closed())
await asyncio.sleep(0)
self.assertFalse(task.done())
srv._attach()
loop = asyncio.get_running_loop()
loop.call_soon(srv.close)
loop.call_soon(srv._detach)
await srv.wait_closed()




@unittest.skipUnless(hasattr(asyncio, 'ProactorEventLoop'), 'Windows only')
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Another attempt at fixing :func:`asyncio.Server.wait_closed()`. It now
blocks until both conditions are true: the server is closed, *and* there
are no more active connections. (This means that in some cases where in
3.12.0 this function would *incorrectly* have returned immediately,
it will now block; in particular, when there are no active connections
but the server hasn't been closed yet.)
Loading