[issue19850] asyncio: limit EINTR occurrences with SA_RESTART

Charles-François Natali added the comment: Thanks! -- resolution: -> fixed stage: patch review -> committed/rejected status: open -> closed ___ Python tracker ___ __

[issue19850] asyncio: limit EINTR occurrences with SA_RESTART

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 _

[issue19850] asyncio: limit EINTR occurrences with SA_RESTART

Guido van Rossum added the comment: OK, I've harassed you enough. Sorry. Go ahead and commit this. -- ___ Python tracker ___ ___ Pytho

[issue19850] asyncio: limit EINTR occurrences with SA_RESTART

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 th

[issue19850] asyncio: limit EINTR occurrences with SA_RESTART

STINNER Victor added the comment: 2013/12/3 Guido van Rossum : > 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 the main thread. On

[issue19850] asyncio: limit EINTR occurrences with SA_RESTART

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 __

[issue19850] asyncio: limit EINTR occurrences with SA_RESTART

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 _

[issue19850] asyncio: limit EINTR occurrences with SA_RESTART

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

[issue19850] asyncio: limit EINTR occurrences with SA_RESTART

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

[issue19850] asyncio: limit EINTR occurrences with SA_RESTART

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 sel

[issue19850] asyncio: limit EINTR occurrences with SA_RESTART

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 "exactl

[issue19850] asyncio: limit EINTR occurrences with SA_RESTART

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

[issue19850] asyncio: limit EINTR occurrences with SA_RESTART

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).

[issue19850] asyncio: limit EINTR occurrences with SA_RESTART

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 _

[issue19850] asyncio: limit EINTR occurrences with SA_RESTART

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

[issue19850] asyncio: limit EINTR occurrences with SA_RESTART

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 "Interru

[issue19850] asyncio: limit EINTR occurrences with SA_RESTART

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

[issue19850] asyncio: limit EINTR occurrences with SA_RESTART

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. : "