Workaround asyncio signal handling on Unix#479
Conversation
Unix asyncio loops override existing signal wakeup file descriptors with no regards for existing ones -- set by user code or by another loop Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
|
I'll open another ticket upstream. |
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
ivanpauno
left a comment
ivanpauno
left a comment
There was a problem hiding this comment.
I'm not really comfortable with the fact that this PR overwrites signal.set_wakeup_fd, but the code looks reasonable and I don't know how to fix this in another way.
It's worth writing a big comment explaining why this is being done in the code.
LGTM with nits addressed.
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
|
@ivanpauno PTAL @ a15958a |
does this PR mean that signal handlers added with asyncio.loop.add_signal_handler are ignored if you later happen to asynchronously launch a process? Really, that's pretty broken 😄 .... |
|
But now that I think about it, I should enforce it for the |
I mean, does something like: loop.add_signal_handler(...)
# first process launch in next line, child watcher initialized there
process = asyncio.create_subprocess_exec(...)work, or is that also broken? If that's not broken, if they would support |
If it's the same loop for both calls, it works. If it's different loops, last one wins :D |
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
|
f90f8bc should take care of multi-threaded scenarios. It's (I think) exception-safe too. |
|
CI up to
|
|
Alright, all's green and reviewer's happy. Going in! |
Unix asyncio loops override existing signal wakeup file descriptors with no regards for existing ones -- set by user code or by another loop Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Unix asyncio loops override existing signal wakeup file descriptors with no regards for existing ones -- set by user code or by another loop Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
* Handle signals within the asyncio loop. (#476) * Workaround asyncio signal handling on Unix (#479) * Remove is_winsock_handle() and instead test if wrapping the handle in a socket.socket() works (#494) * Close the socket pair used for signal management (#497) * Only try to wrap the fd in a socket on Windows (#498) * Bring back deprecated launch.utilities.signal_management APIs Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com> Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
2 participants
Unix
asyncioloops override existing signal wakeup file descriptors with no regards for existing ones -- set by user code or by another loop. For further reference, see here. This is a problem when the child watcher gets lazy initialized (e.g. on first async subprocess execution, see here and here).CI up to
launch: