Repository navigation
asyncio.Server.wait_closed() appears to hang indefinitely in 3.12.0a7 #104344
Copy link
Copy link
Closed
christgau/wsdd
#219Labels
Description
Bug report
I have some code in the qemu.qmp package that appears to work correctly in Python 3.6 (RIP~) through to Python 3.11. I've started testing against 3.12 and I'm finding that waiting on asyncio.Server.wait_closed() is creating a deadlock/hang.
Here's a minimal-ish reproducer:
#!/usr/bin/env python3
import asyncio
async def cause_problems():
accepted = asyncio.Event()
async def _incoming(reader, writer):
print("entered _incoming")
print("signaling outer context; we accepted a client")
accepted.set()
print("left _incoming")
print("starting server")
server = await asyncio.start_unix_server(
_incoming,
path="problems.sock",
backlog=1,
)
print("created server; waiting on a client")
await accepted.wait()
print("got a client")
print("closing server")
server.close()
print("issued close()")
await server.wait_closed()
print("finished waiting on server to close")
if __name__ == '__main__':
asyncio.run(cause_problems())You can test a simple connection using whatever you'd like; socat works to trigger the accept code, e.g. from the same directory that you run the above code, try:
socat UNIX-CONNECT:problems.sock stdoutHere's what happens in python 3.11.3
jsnow@scv ~> python3.11 hang.py
starting server
created server; waiting on a client
entered _incoming
signaling outer context; we accepted a client
left _incoming
got a client
closing server
issued close()
finished waiting on server to close
And here's 3.12.0a7:
jsnow@scv ~> python3.12 hang.py
starting server
created server; waiting on a client
entered _incoming
signaling outer context; we accepted a client
left _incoming
got a client
closing server
issued close()
When I issue ^C, we get this traceback:
^CTraceback (most recent call last):
File "/usr/lib64/python3.12/asyncio/runners.py", line 118, in run
return self._loop.run_until_complete(task)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib64/python3.12/asyncio/base_events.py", line 664, in run_until_complete
return future.result()
^^^^^^^^^^^^^^^
File "/home/jsnow/hang2.py", line 26, in cause_problems
await server.wait_closed()
File "/usr/lib64/python3.12/asyncio/base_events.py", line 384, in wait_closed
await waiter
asyncio.exceptions.CancelledError
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/home/jsnow/hang2.py", line 31, in <module>
asyncio.run(cause_problems())
File "/usr/lib64/python3.12/asyncio/runners.py", line 194, in run
return runner.run(main)
^^^^^^^^^^^^^^^^
File "/usr/lib64/python3.12/asyncio/runners.py", line 123, in run
raise KeyboardInterrupt()
KeyboardInterrupt
Your environment
- Python 3.12.0a7 (as packaged on Fedora 37)
Linux 6.2.12-200.fc37.x86_64 #1 SMP PREEMPT_DYNAMIC Thu Apr 20 23:38:29 UTC 2023- Tested on CPython 3.6 through 3.12 as available on Fedora 37. Works on 3.6, 3.7, 3.8, 3.9, 3.10 and 3.11 but fails on the 3.12 alpha package.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
Projects
- StatusShow more project fieldsDone
Activity
kumaraditya303 commentedon May 10, 2023
Why aren't you closing the writer in
_incoming? That fixes the problem for me and is bug in your code.jnsnow commentedon May 10, 2023
For python 3.6 through 3.11 you don't have to close the writer to stop the server: you can accept a single peer, stop the server, and continue communicating with the peer.
bonzini commentedon May 10, 2023
So something like this?
Indeed the docs don't say that
Server.wait_closed()waits for all writers (and I don't see why it would).kumaraditya303 commentedon May 10, 2023
cc @gvanrossum bisect points to 5d09d11
bonzini commentedon May 10, 2023
Looks like this was done on purpose for GH-79033, though.
gvanrossum commentedon May 10, 2023
So I think indeed that the better semantics are for
wait_closed()to wait until all requests have been handled, i.e. "draining" the request queue, while not accepting further requests. That makes it useful, as opposed to it being a no-op as it was before.I'm a little surprised that the request handler (
_incomingin @jnsnow's example) is required to close the connection, but it sounds like that is not something we should change (given the possibility of the reader and writer surviving the handler, as in @bonzini's example).In a production version of @jnsnow's example, wouldn't it be leaking sockets if the handler never closed its writer?
gvanrossum commentedon May 10, 2023
(Unless there's a better argument why GH-79033 is wrong, we should just document things better.)
jnsnow commentedon May 10, 2023
In the production code, we save the reader/writer for communicating with the peer. The intent of the code as it stands in the actual library is: "Accept precisely one peer, stop accepting new peers, then communicate via reader/writer."
(My example in the report was just the shortest reproducer I could distill.)
Presumably we don't leak a socket because we close the reader/writer on disconnect.
bonzini commentedon May 10, 2023
I agree that using
wait_closed()is pointless both in @jnsnow's minimal example and in mine, and it was working just because the code was incorrect.On the other hand the usage is exactly what you would expect when reading the documentation for
wait_closed(), which says nothing about waiting for clients: it's just "Wait until the close() method completes".What about deprecating
Server.wait_closed()and putting the new implementation in a new method, likeServer.wait_clients()?gvanrossum commentedon May 10, 2023
Maybe, but looking at the implementation of
wait_closed(), the intention was always to wait for the last connection to be closed. The documentation wasn't super clear about this, but that is clearly what it was meant to do, so I think it's reasonable to fix the bug without renaming the code. We should also clarify the docs (I wouldn't even mind adding a "version-changed: 3.12" note reminding users that this was broken until 3.11). The examples were just buggy.29 remaining items
Make sure clients are closed before the server
Make sure clients are closed before the server
Make sure clients are closed before the server
Make sure clients are closed before the server
Make sure clients are closed before the server (#200)
Make sure clients are closed before the server (#200)
python/qmp: remove Server.wait_closed() call for Python 3.12
python/qmp: remove Server.wait_closed() call for Python 3.12
python/qmp: remove Server.wait_closed() call for Python 3.12
kumaraditya303 commentedon Jul 25, 2026
I created #154676 to add the version changed note.
tests: fix TestLNTransport.test_loop for python 3.12
fix: force-close active connections on shutdown to prevent hang
python/qmp: remove Server.wait_closed() call for Python 3.12