Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 2655369

Browse filesBrowse files
gvanrossumwillingc
andauthored
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
Copy full SHA for 2655369

File tree

4 files changed

+67
-9
lines changed
Filter options

4 files changed

+67
-9
lines changed

‎Doc/library/asyncio-eventloop.rst

Copy file name to clipboardExpand all lines: Doc/library/asyncio-eventloop.rst
+5-3Lines 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

Copy file name to clipboardExpand all lines: Lib/asyncio/base_events.py
+22-2Lines 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

Copy file name to clipboardExpand all lines: Lib/test/test_asyncio/test_server.py
+34-4Lines 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')
+6Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.