Roundup Robot added the comment:
New changeset 546cad3627e2 by Charles-François Natali in branch 'default':
Issue #19850: asyncio: Set SA_RESTART when registering a signal handler to
http://hg.python.org/cpython/rev/546cad3627e2
--
nosy: +python-dev
Charles-François Natali added the comment:
Thanks!
--
resolution: - fixed
stage: patch review - committed/rejected
status: open - closed
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19850
Anthony Baire added the comment:
The patch is fine, but it is hard to rely on it to prevent bugs from happening
because that requires cooperation from all modules registering signal handlers.
Anyway it facilitates reusing code that was not written for an event-driven
context (and many will do
STINNER Victor added the comment:
SA_RESTART doesn't need to be enforced. It's better to use it, but
selectors and asyncio modules already handle EINTR error.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19850
Guido van Rossum added the comment:
Please answer this question. Under what circumstances can a signal handler
interrupt a blocking system call in a thread that is not the main thread?
--
___
Python tracker rep...@bugs.python.org
STINNER Victor added the comment:
2013/12/3 Guido van Rossum rep...@bugs.python.org:
Please answer this question. Under what circumstances can a signal handler
interrupt a blocking system call in a thread that is not the main thread?
There is no guarantee that the signal handler is called in
Charles-François Natali added the comment:
The patch is fine, but it is hard to rely on it to prevent bugs from
happening because that requires cooperation from all modules registering
signal handlers.
Once again, that's why the bug report says *limit* EINTR occurrences.
We all know this
Guido van Rossum added the comment:
OK, I've harassed you enough. Sorry. Go ahead and commit this.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19850
___
STINNER Victor added the comment:
asyncio_sa_restart.diff looks good to me, it cannot make the situation worse.
signal.siginterrupt() looks to be available on all non-Windows buildbots and
working correctly: test_signal tests it and the test pass.
--
nosy: +haypo
Guido van Rossum added the comment:
I have a question. Is there actually any need for this with asyncio? The
selector already handles EINTR, and all the I/O done by asyncio's transports
already handles it too (there are except (BlockingIOError, InterruptedError)
clauses all over the place).
STINNER Victor added the comment:
I have a question. Is there actually any need for this with asyncio?
I believe that the libc and the kernel knows better than Python how to
restart a syscalls, than Python. I expect more reliable timeout, or
the kernel may avoid context switches (don't wake up
Guido van Rossum added the comment:
That's just rhetoric -- I am beginning to believe that nobody has any data.
Here's some opposite rhetoric. If it ain't broke, don't fix it. Plus, if it's
so much better, why isn't it the default? If you say for backward
compatibility I will say exactly!
Gregory P. Smith added the comment:
I believe that the libc and the kernel knows better than Python how to
restart a syscalls, than Python. I expect more reliable timeout, or
the kernel may avoid context switches (don't wake up the process).
See the man page i linked to, calls like select
Charles-François Natali added the comment:
Guido van Rossum added the comment:
That's just rhetoric -- I am beginning to believe that nobody has any data.
Here's some opposite rhetoric. If it ain't broke, don't fix it. Plus, if
it's so much better, why isn't it the default? If you say
New submission from Charles-François Natali:
asyncio makes heavy use of SIGCHLD for subprocesses.
A consequence of this if that a lot of syscalls can fail with EINTR (see e.g.
issue #18885).
The attached patch uses SA_RESTART (through signal.siginterrupt()) to limit
EINTR occurrences, e.g. :
Guido van Rossum added the comment:
Do you haven an example of a program using asyncio that fails because of this?
Adding gps because he seems to agree that EINTR must die.
--
nosy: +gregory.p.smith
___
Python tracker rep...@bugs.python.org
Gregory P. Smith added the comment:
It sounds like doing this is fine (as Glyph suggests in the email thread) but
it isn't magically going to solve all EINTR problems, just reduce the number of
calls that could encounter them.
http://man7.org/linux/man-pages/man7/signal.7.html see the
Charles-François Natali added the comment:
It sounds like doing this is fine (as Glyph suggests in the email thread) but
it isn't magically going to solve all EINTR problems, just reduce the number
of calls that could encounter them.
Indeed, hence *limit* EINTR occurrences :-)
Note that
18 matches
Mail list logo
