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 python3importasyncioasyncdefcause_problems():
accepted=asyncio.Event()
asyncdef_incoming(reader, writer):
print("entered _incoming")
print("signaling outer context; we accepted a client")
accepted.set()
print("left _incoming")
print("starting server")
server=awaitasyncio.start_unix_server(
_incoming,
path="problems.sock",
backlog=1,
)
print("created server; waiting on a client")
awaitaccepted.wait()
print("got a client")
print("closing server")
server.close()
print("issued close()")
awaitserver.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 stdout
Here'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.
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.
Reacted by Paolo Bonzini, Mikhail Koviazin and Efesto
Looks like this was done on purpose for GH-79033, though.
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 (_incoming in @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?
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.
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, like Server.wait_clients()?
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.
Activity
kumaraditya303 commented on 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 commented on 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 commented on 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 commented on May 10, 2023
cc @gvanrossum bisect points to 5d09d11
bonzini commented on May 10, 2023
Looks like this was done on purpose for GH-79033, though.
gvanrossum commented on 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 (_incoming in @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 commented on May 10, 2023
(Unless there's a better argument why GH-79033 is wrong, we should just document things better.)
jnsnow commented on 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 commented on 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, like Server.wait_clients()?
gvanrossum commented on 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 commented on 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