You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This test occasionally fails with a timeout. Old comments indicate that this was once considered a pypy-specific issue although it has recently been observed on cpython 3.10. It is unclear whether this is a real bug or an issue with the test, although I suspect it's a real bug.
Debug logging indicates that the child process is in fact closing its stdout, suggesting that the problem is in the signal handling on the parent process side. Brainstorming:
We use asyncio's call_soon_threadsafe from a signal handler. This method is not documented to be signal-safe, although it is used in this way in asyncio-internal code. (This code is not used by default in python 3.8+, although it looks like it was in older versions)
We silently ignore ChildProcessError, which could mask problems. (Note that logged timing data does not suggest that the child process terminated on its own before the signal was sent)
Aside from fixing this bug, it would be nice if we could adopt pidfd instead of SIGCHLD. asyncio already has support for this; we should see if we could use that instead of building our own.
The text was updated successfully, but these errors were encountered:
This method is not documented to be signal-safe, although it is used in this way in asyncio-internal code.
I think I was misreading this asyncio-internal code. It was not in fact being called from a signal handler, but via the event loop's loop.add_signal_handler, which runs the callback on the event loop instead of in a signal handler. So add_callback_from_signal has probably been a little broken ever since we switched to the asyncio event loop. I'm going to deprecate and remove it.
bdarnell
added a commit
to bdarnell/tornado
that referenced
this issue
May 2, 2023
I don't believe this method is currently working as intended, and I'm
not sure it ever has since the move to asyncio. I think this is
responsible for occasional test failures in CI.
Fixestornadoweb#3225
I don't believe this method is currently working as intended, and I'm
not sure it ever has since the move to asyncio. I think this is
responsible for occasional test failures in CI.
Fixestornadoweb#3225
This test occasionally fails with a timeout. Old comments indicate that this was once considered a pypy-specific issue although it has recently been observed on cpython 3.10. It is unclear whether this is a real bug or an issue with the test, although I suspect it's a real bug.
Debug logging indicates that the child process is in fact closing its stdout, suggesting that the problem is in the signal handling on the parent process side. Brainstorming:
call_soon_threadsafe
from a signal handler. This method is not documented to be signal-safe, although it is used in this way in asyncio-internal code. (This code is not used by default in python 3.8+, although it looks like it was in older versions)Aside from fixing this bug, it would be nice if we could adopt
pidfd
instead ofSIGCHLD
.asyncio
already has support for this; we should see if we could use that instead of building our own.The text was updated successfully, but these errors were encountered: