This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: asyncio.ProactorEventLoop mishandles signal wakeup file descriptor
Type: behavior Stage:
Components: asyncio, Windows Versions: Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: asvetlov, gvanrossum, hidmic, paul.moore, steve.dower, tim.golden, yselivanov, zach.ware
Priority: normal Keywords:

Created on 2021-01-12 21:43 by hidmic, last changed 2022-04-11 14:59 by admin.

Messages (6)
msg384979 - (view) Author: Michel Hidalgo (hidmic) Date: 2021-01-12 21:43
asyncio.ProactorEventLoop uses a socket.socketpair and signal.set_wakeup_fd to wake up a loop that's polling I/O. However it does so with no consideration for file descriptors previously set (i.e. no signal number forwarding). Either by user code or by another instance of asyncio.ProactorEventLoop.

The following snippet is enough for the above to cause the loop to hang forever:

import asyncio
import gc

asyncio.set_event_loop(asyncio.ProactorEventLoop())
asyncio.set_event_loop(asyncio.ProactorEventLoop())
gc.collect()
asyncio.get_event_loop().run_forever()


The first asyncio.ProactorEventLoop instance sets a signal wakeup file descriptor on construction (see https://github.com/python/cpython/blob/187f76def8a5bd0af7ab512575cad30cfe624b05/Lib/asyncio/proactor_events.py#L632). The second instances does the same, dropping the file descriptor set by the first one (not good, not necessarily bad). When the garbage collector purges the first instance, signal wakeups are disabled completely (see https://github.com/python/cpython/blob/187f76def8a5bd0af7ab512575cad30cfe624b05/Lib/asyncio/proactor_events.py#L679). The loop cannot be interrupted with Ctrl+C anymore (bad).
msg384998 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-01-13 06:26
Looks this was not tested or thought through with multiple loops and signals (admittedly, using signals is never fun, and even less so on Windows).

Can you send a PR with a fix?
msg385198 - (view) Author: Michel Hidalgo (hidmic) Date: 2021-01-18 14:06
Sorry for taking so long to reply. 

Sure, I would be happy to contribute. We should probably take care of Unix event loops -- since I opened this ticket I found out those tend to not cooperate with other signal wakeup file descriptor users either. I am a bit short on time right now though, so it will take some time.
msg385211 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-01-18 18:09
No pressure. If there's an API change needed you will have until 3.10 beta 1. Without API changes this can be fixed any time.
msg398445 - (view) Author: Michel Hidalgo (hidmic) Date: 2021-07-28 23:56
Circling back. I've been giving some thought to this. While this could be fixed within asyncio.proactor_events.ProactorEventLoop and asyncio.unix_events._UnixSelectorEventLoop implementations (e.g. calling signal.set_wakeup_fd once and forwarding signal numbers to subsequent instances through file descriptors or direct reference), I can't help thinking I've encountered this issue of contending signal API calls too many times already. Rarely two call sites cooperate with each other.

So... how do people feel about changing the signal API module to enable multiple file descriptors and multiple signal handlers per signal? It'd be much simpler to use, and not only in asyncio. Implementation-wise, a lock-free singly linked list could be used to store them (may need to expose compare-and-swap atomic primitives in pycore_atomic.h).
msg398457 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-07-29 05:27
Have you investigated how that could be implemented and what the new API would look like? Personally I don't really like signals (they're not portable and code using them is super hard to get right) and I hesitate to add to their API. OTOH maybe you'll be able to find a champion for such a change among other core devs. I doubt that anyone is going to put any time in this based on a few messages from you though -- you must do the work.
History
Date User Action Args
2022-04-11 14:59:40adminsetgithub: 87079
2021-07-29 05:27:36gvanrossumsetmessages: + msg398457
2021-07-28 23:56:14hidmicsetmessages: + msg398445
2021-01-18 18:09:15gvanrossumsetmessages: + msg385211
2021-01-18 14:07:00hidmicsetmessages: + msg385198
2021-01-13 06:26:33gvanrossumsetnosy: + gvanrossum
messages: + msg384998
2021-01-12 21:43:53hidmiccreate