Skip to content

Commit 2655369

Browse files
gvanrossumwillingc
gvanrossum
and
willingc
authored
gh-79033: Try to fix asyncio.Server.wait_closed() again (GH-111336)
* Try to fix asyncio.Server.wait_closed() again I identified the condition that `wait_closed()` is intended to wait for: the server is closed *and* there are no more active connections. When this condition first becomes true, `_wakeup()` is called (either from `close()` or from `_detach()`) and it sets `_waiters` to `None`. So we just check for `self._waiters is None`; if it's not `None`, we know we have to wait, and do so. A problem was that the new test introduced in 3.12 explicitly tested that `wait_closed()` returns immediately when the server is *not* closed but there are currently no active connections. This was a mistake (probably a misunderstanding of the intended semantics). I've fixed the test, and added a separate test that checks exactly for this scenario. I also fixed an oddity where in `_wakeup()` the result of the waiter was set to the waiter itself. This result is not used anywhere and I changed this to `None`, to avoid a GC cycle. * Update Lib/asyncio/base_events.py --------- Co-authored-by: Carol Willing <carolcode@willingconsulting.com>
1 parent 9d4a1a4 commit 2655369

4 files changed

Lines changed: 67 additions & 9 deletions

File tree

‎Doc/library/asyncio-eventloop.rst‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1619,8 +1619,9 @@ Do not instantiate the :class:`Server` class directly.
16191619
The sockets that represent existing incoming client connections
16201620
are left open.
16211621

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

16251626
.. method:: get_loop()
16261627

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

16791680
.. coroutinemethod:: wait_closed()
16801681

1681-
Wait until the :meth:`close` method completes.
1682+
Wait until the :meth:`close` method completes and all active
1683+
connections have finished.
16821684

16831685
.. attribute:: sockets
16841686

‎Lib/asyncio/base_events.py‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ def _wakeup(self):
305305
self._waiters = None
306306
for waiter in waiters:
307307
if not waiter.done():
308-
waiter.set_result(waiter)
308+
waiter.set_result(None)
309309

310310
def _start_serving(self):
311311
if self._serving:
@@ -377,7 +377,27 @@ async def serve_forever(self):
377377
self._serving_forever_fut = None
378378

379379
async def wait_closed(self):
380-
if self._waiters is None or self._active_count == 0:
380+
"""Wait until server is closed and all connections are dropped.
381+
382+
- If the server is not closed, wait.
383+
- If it is closed, but there are still active connections, wait.
384+
385+
Anyone waiting here will be unblocked once both conditions
386+
(server is closed and all connections have been dropped)
387+
have become true, in either order.
388+
389+
Historical note: In 3.11 and before, this was broken, returning
390+
immediately if the server was already closed, even if there
391+
were still active connections. An attempted fix in 3.12.0 was
392+
still broken, returning immediately if the server was still
393+
open and there were no active connections. Hopefully in 3.12.1
394+
we have it right.
395+
"""
396+
# Waiters are unblocked by self._wakeup(), which is called
397+
# from two places: self.close() and self._detach(), but only
398+
# when both conditions have become true. To signal that this
399+
# has happened, self._wakeup() sets self._waiters to None.
400+
if self._waiters is None:
381401
return
382402
waiter = self._loop.create_future()
383403
self._waiters.append(waiter)

‎Lib/test/test_asyncio/test_server.py‎

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,29 +122,59 @@ async def main(srv):
122122

123123
class TestServer2(unittest.IsolatedAsyncioTestCase):
124124

125-
async def test_wait_closed(self):
125+
async def test_wait_closed_basic(self):
126126
async def serve(*args):
127127
pass
128128

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

131-
# active count = 0
132+
# active count = 0, not closed: should block
132133
task1 = asyncio.create_task(srv.wait_closed())
133134
await asyncio.sleep(0)
134-
self.assertTrue(task1.done())
135+
self.assertFalse(task1.done())
135136

136-
# active count != 0
137+
# active count != 0, not closed: should block
137138
srv._attach()
138139
task2 = asyncio.create_task(srv.wait_closed())
139140
await asyncio.sleep(0)
141+
self.assertFalse(task1.done())
140142
self.assertFalse(task2.done())
141143

142144
srv.close()
143145
await asyncio.sleep(0)
146+
# active count != 0, closed: should block
147+
task3 = asyncio.create_task(srv.wait_closed())
148+
await asyncio.sleep(0)
149+
self.assertFalse(task1.done())
144150
self.assertFalse(task2.done())
151+
self.assertFalse(task3.done())
145152

146153
srv._detach()
154+
# active count == 0, closed: should unblock
155+
await task1
147156
await task2
157+
await task3
158+
await srv.wait_closed() # Return immediately
159+
160+
async def test_wait_closed_race(self):
161+
# Test a regression in 3.12.0, should be fixed in 3.12.1
162+
async def serve(*args):
163+
pass
164+
165+
srv = await asyncio.start_server(serve, socket_helper.HOSTv4, 0)
166+
self.addCleanup(srv.close)
167+
168+
task = asyncio.create_task(srv.wait_closed())
169+
await asyncio.sleep(0)
170+
self.assertFalse(task.done())
171+
srv._attach()
172+
loop = asyncio.get_running_loop()
173+
loop.call_soon(srv.close)
174+
loop.call_soon(srv._detach)
175+
await srv.wait_closed()
176+
177+
148178

149179

150180
@unittest.skipUnless(hasattr(asyncio, 'ProactorEventLoop'), 'Windows only')

‎Misc/NEWS.d/next/Library/2023-10-25-11-54-00.gh-issue-79033.5ePgFl.rst‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
Another attempt at fixing :func:`asyncio.Server.wait_closed()`. It now
2+
blocks until both conditions are true: the server is closed, *and* there
3+
are no more active connections. (This means that in some cases where in
4+
3.12.0 this function would *incorrectly* have returned immediately,
5+
it will now block; in particular, when there are no active connections
6+
but the server hasn't been closed yet.)

0 commit comments

Comments
 (0)