As discussed in http://mail.python.org/pipermail/python-dev/2013-August/128204.html, I think that we shouldn't let EINTR leak to Python code: it should be handled properly by the C code, so that users (and the Python part of the stdlib) don't have to worry about this low-level historical nuisance.

For code that doesn't release the GIL, we could simply use this glibc macro:
# define TEMP_FAILURE_RETRY(expression) \
  (__extension__                                                              \
    ({ long int __result;                                                     \
       do __result = (long int) (expression);                                 \
       while (__result == -1L && errno == EINTR);                             \
       __result; }))
#endif

Now, I'm not sure about how to best handle this for code that releases the GIL.

Basically:

    Py_BEGIN_ALLOW_THREADS
    pid = waitpid(pid, &status, options);
    Py_END_ALLOW_THREADS

should become

begin_handle_eintr:
        Py_BEGIN_ALLOW_THREADS
        pid = waitpid(pid, &status, options);
        Py_END_ALLOW_THREADS

        if (pid < 0 && errno == EINTR) {
            if (PyErr_CheckSignals())
                return NULL;
            goto begin_handle_eintr;
        }

Should we do this with a macro?

If yes, should it be a new one that should be placed around Py_BEGIN_ALLOW_THREADS/Py_END_ALLOW_THREADS (like BEGIN_SELECT_LOOP in selectmodule.c) or could we have a single macro that would do both (i.e. release the GIL / reacquire the GIL, and try again in case of EINTR, unless a signal handler raised an exception)?

From a cursory look, the main files affected would be:
Modules/fcntlmodule.c
Modules/ossaudiodev.c
Modules/posixmodule.c
Modules/selectmodule.c
Modules/selectmodule.c
Modules/signalmodule.c
Modules/socketmodule.c
Modules/syslogmodule.c

FYI - use the changes made in http://bugs.python.org/issue12268 as a guide for how to deal with EINTR properly at the C level.  See the _PyIO_trap_eintr() function for example.

See also _eintr_retry_call() in Lib/subprocess.py.

FWIW, there are times when we *want* the interrupted system call to return control to Python rather than retrying the call.  If someone is making a Python equivalent of the low level system call such as select() or poll(), the EINTR should be exposed for Python code to handle.

Things like time.sleep() are documented as sleeping for less time when a signal has arrived even though an exception may not be raised. People have written code which depends on this behavior so adding an EINTR retry for the remaining sleep time would break some programs.

Getting an EINTR errno does *not* mean you can simply retry the system calls with the exact same arguments.  ie: If you did that with the select() call within time.sleep it'd be trivial to make the process sleep forever by sending it signals with a frequency less than the sleep time.

Gregory, thanks, that's what I was planning to do.

But since the recent discussions (mainly on selectors), there are points I obviously don't - and won't - agree with (such as select() returning EINTR or returning early, same for sleep()), I'm not interested in this anymore.
Anyone interested can pick this up, though.

(BTW, as for applications relying on EINTR being returned, I'm positive *way more applications* will break because of the recent change making file descriptors close-on-exec by default).

> FWIW, there are times when we *want* the interrupted system call to
> return control to Python rather than retrying the call.

I'm a bit curious, do you know of any use cases?

> If someone is making a Python equivalent of the low level system call
> such as select() or poll(), the EINTR should be exposed for Python
> code to handle.

As mentioned in another issue, you would use a special wakeup fd to
wakeup select() or poll() calls.

> Getting an EINTR errno does *not* mean you can simply retry the system
> calls with the exact same arguments.  ie: If you did that with the
> select() call within time.sleep it'd be trivial to make the process
> sleep forever by sending it signals with a frequency less than the
> sleep time.

Indeed. That's already done in e.g. socketmodule.c : take a look at the
BEGIN_SELECT_LOOP / END_SELECT_LOOP macros.

On Sat, Aug 31, 2013 at 9:56 AM, Charles-François Natali
<report@bugs.python.org> wrote:
>
> Charles-François Natali added the comment:
>
> Gregory, thanks, that's what I was planning to do.
>
> But since the recent discussions (mainly on selectors), there are points I obviously don't - and won't - agree with (such as select() returning EINTR or returning early, same for sleep()), I'm not interested in this anymore.

Whoa. Maybe you're overreacting a bit? I personally see a big divide
here between system calls whose functionality includes sleeping (e.g.
sleep(), poll(), select()) and those that just want some I/O to
complete (e.g. recv(), send(), read(), write()). The former are almost
always used in a context that can handle premature returns just fine,
since the return value for a premature return is the same as for
hitting the deadline, and the timeout is often used just as a hint
anyway. It's the latter category (recv() etc.) where the EINTR return
is problematic, and I think for many of those the automatic retry
(after the Python-level signal handler has been run and conditional on
it not raising an exception) will be a big improvement.

> Anyone interested can pick this up, though.
>
> (BTW, as for applications relying on EINTR being returned, I'm positive *way more applications* will break because of the recent change making file descriptors close-on-exec by default).

Again, I'd make a distinction: I agree for send(), recv() etc., but I
don't think there are many buggy uses of select()/poll() timeouts
around. (And even if there are, I still think it's better to fix these
by correcting the retry logic in the framework or the application,
since it may have other considerations.)

I wrote too many words.  In short:

time.sleep()'s behavior should remain as it is today given how it is documented to behave.  If you disagree, consider adding an optional interruptable=False parameter so that both behavior options exist.

ALL IO calls and wait* should handle EINTR transparently for the user and never expose it to the Python application.

select(), poll() and equivalents.  If you want to transparently handle EINTR on these, just make sure you deal with the timeouts properly.  While I suspect a few people wanted to see the signal interruption on those I agree: very uncommon and undesirable for most.

If people need a specific signal interruption they should define a signal handler that raises.

(replying to Guido's post in another thread)

> Charles-Francois, sorry to add you back to the bug, but (a) I thought you had agreed to a compromise patch that restarts signals in most cases but not for select(), poll() etc.; (b) I may have found a flaw in the idea.
> The flaw (if it is one) is related to Py_AddPendingCall(). This "schedules" a pending callback, mostly for signals, but doesn't AFAICT interrupt the mainthread in any way. (TBH, I only understand the code for Python 2.7, and in that version I'm sure it doesn't.)
>
> So is this a flaw? I'm nor sure. Can you think about it?

I don't think that's a problem: the way I was planning to tackle signals is to call PyErr_CheckSignals() before retrying upon EINTR: this runs signal handlers, and returns a non 0 value if an exception occured (e.g. KeyboardInterrupt): if that's the case, then we simply break out of the loop, and let the exception bubble up.
See e.g. http://hg.python.org/cpython/file/default/Modules/socketmodule.c#l3397

Alright, here's a first step: select/poll/epoll/etc now return empty
lists/tuples upon EINTR. This comes with tests (note that all those tests
could probably be factored, but that's another story).

Am I correct in thinking that you're simply replacing the OSError(EINTR) with returning empty lists?  This is bound to subtly break code, e.g. the code that expects reasonably that a return value of three empty lists means the timeout really ran out (i.e. the version of the code that is already the most careful).  Shouldn't you restart the poll with the remaining time until timeout?

I wouldn't call that "being the most careful".  I've always had an implicit understanding that calls with timeouts may, for whatever reason, return sooner than requested (or later!), and the most careful approach is to re-check the clock again.

> I've always had an implicit understanding that calls with timeouts may,
> for whatever reason, return sooner than requested (or later!), and the
> most careful approach is to re-check the clock again.

I've always had the implicit understanding that if I use an *infinite* timeout then the call will not timeout.

> > I've always had an implicit understanding that calls with timeouts may,
> > for whatever reason, return sooner than requested (or later!), and the
> > most careful approach is to re-check the clock again.
> 
> I've always had the implicit understanding that if I use an *infinite*
> timeout then the call will not timeout.

Wow, that's a good point. select() and friends are not documented to
exhibit successful spurious wakeups. It would be a pretty strong
compatibility breach if they started doing so.

If we don't want select() to silently retry on EINTR, then I think we
should leave it alone.

Speaking of which, I see that SelectSelector.select() returns an empty
list when interrupted, but this is nowhere documented.

> I've always had an implicit understanding that calls with timeouts may, for whatever reason, return sooner than requested (or later!), and the most careful approach is to re-check the clock again.

exactly.  at the system call level you can be interrupted.  re-checking the clock is the right thing to do if the elapsed time actually matters.

> If we don't want select() to silently retry on EINTR, then I think we
should leave it alone.

We should go ahead and retry for the user for select/poll/epoll/kqueue.  If they care about being able to break out of that low level call due to a signal, they should set a signal handler which raises an exception.  I have *never* seen code intentionally get an EINTR exception from a select or poll call and have often seen code tripped up because it or a library it was using forgot to handle it.

We're a high level language: Lets be sane by default and do the most desirable thing for the user. Retry the call internally with a safely adjusted timeout:
  new_timeout = min(original_timeout, time_now-start_time)
  if new_timeout <= 0:
    return an empty list  # ie: the system clock changed
  retry the call with new_timeout

We went through this whole discussion before.  Returning immediately with three empty lists is better than raising InterruptedError.  Retrying is not always better.

Modules/socketmodule.c is using a simple style to implement socket timeouts using select().  If I were to naively copy this style over to pure Python, it would work in current Pythons; I'd get occasionally an OSError(EINTR), which I would have presumably been annoyed with and am now catching properly.  Now if my working code was made to run with a select() modified as proposed, an EINTR would instead cause the program to fail more obscurely: its sockets occasionally -- and apparently without reason -- time out much earlier.  In that situation I would have a hard time finding the reason, particularly if running on an OS where the system select() doesn't spuriously return early with a timeout ("man select" on Linux guarantees this, for example).

Similarly, an existing program might rely on select() with an infinite timeout to only return when one of the descriptors is ready, particularly if called with only one or two descriptors.

Overall, I would far prefer the status quo over a change in the logic from one slightly-subtle situation to another differently slightly-subtle one.  I believe this would end up with programs that need to take special care about both kinds of subtlenesses just to run on two versions of Python.  I may be wrong, in this case sorry to take your time. :-)

Guido's point was that it is already a bug in code to not check the elapsed
time after a select call returns rather than assuming the full timeout time
has elapsed. Correct code today already needs to deal with both situations
(OSError(EINTR) and select returning an empty set before the desired time
has elapsed) because both can happen on existing systems today. So correct
code in the future wishing to be compatible with older Pythons will need to
continue to do so.

As for "presumably have been annoyed by the occasional OSError(EINTR) and
fix that bug" that isn't always true. EINTRs are not guaranteed to happen
and are likely to crop up on different systems (production systems) long
after you've deployed and successfully run your code as they are something
that happens due to things _outside_ of the control of your deployed
program: signals.

That's what has gotten me on a kick to hide EINTR from python developers
when at all possible.

For the record: I am perfectly fine with select and friends returning an
empty set early on EINTR (as Guido seems to prefer). If this worries some
people lets at least highlight this in the documentation as part of this
change.

What I don't want is to ever see OSError(EINTR) in the future.

Just for the record, I was initially in favor of recomputing the
timeout and retrying upon EINTR, but Guido prefers to return empty
lists, and since that's a better compromise than the current situation
(I've seen *many* people complaining on EINTR popping-up at random
points in the code, including myself), I went ahead and implemented
it.

AFAICT, an early return for calls such as poll()/epoll() etc is
something which is definitely acceptable: if you have a look at e.g.
Tornado, Twisted & Co, they all return empty lists on EINTR.

> I've always had the implicit understanding that if I use an *infinite* timeout then > the call will not timeout.

Well, I've always assumed that time.sleep(n) would sleep n seconds, but:
"""
static int
floatsleep(double secs)
[...]
    Py_BEGIN_ALLOW_THREADS
    err = select(0, (fd_set *)0, (fd_set *)0, (fd_set *)0, &t);
    Py_END_ALLOW_THREADS
    if (err != 0) {
#ifdef EINTR
        if (errno == EINTR) {
            if (PyErr_CheckSignals())
                return -1;
        }
        else
#endif
        {
            PyErr_SetFromErrno(PyExc_IOError);
            return -1;
        }
    }
[...]
"""

So really, I'm like Gregory: I don't care which solution we chose, but
I just don't want to have to let the user handle EINTR.

> Guido's point was that it is already a bug in code to not check the elapsed
> time after a select call returns rather than assuming the full timeout time
> has elapsed.

I don't understand how it's a bug. You're assuming select() has
unreliable timing, but it doesn't (if you are using the same clock).

On dim., 2013-12-01 at 08:14 +0000, Charles-François Natali wrote:
> So really, I'm like Gregory: I don't care which solution we chose, but
> I just don't want to have to let the user handle EINTR.

Well this is wishing thinking, since by returning an empty list you
force the user to handle EINTR - just in a different way.

> Well this is wishing thinking, since by returning an empty list you
> force the user to handle EINTR - just in a different way.

I know that returning an empty list changes the semantics: I just
think that's better - or not as bad - than the current possibility of
having any single piece of code possibly die upon EINTR.

If you want to implement retry with timeout re-computation, I'm not
the one to who must be convinced :-)

(BTW, if we go this way, then time.sleep() should probably also be
fixed to retry upon EINTR).

> I know that returning an empty list changes the semantics: I just
> think that's better - or not as bad - than the current possibility of
> having any single piece of code possibly die upon EINTR.
> 
> If you want to implement retry with timeout re-computation, I'm not
> the one to who must be convinced :-)

Or, since we now have the selectors module, we could let select() live
with the current semantics.

By the way, it's already too late for 3.4, which is in feature freeze.

I do not consider this a feature; that EINTR is exposed as an exception from the API is a bug.  But Larry is the only one who can actually make that decision as the 3.4 release manager (+nosy'd).

> by returning an empty list you force the user to handle EINTR -
> just in a different way.

The user now only has one thing to deal with instead of two: an empty list being returned; something they should already have been dealing with. Gone will be the OSError(EINTR) exception as a rare, often never tested for, alternate form of the same retry needed indication.

I never see code intentionally wanting to receive and handle an OSError(EINTR) exception but I constantly run into code that is buggy due to some library it is using not getting this right... Where it isn't up to the code exhibiting the problem because the only place to fix it is within the library they use that is outside of that code's control.

We've got the opportunity to fix this nit once and for all here, lets do it.

> I do not consider this a feature; that EINTR is exposed as an
> exception from the API is a bug.

select() currently works as specified; you are proposing a
compatibility-breaking change to the API, not a bugfix.

We're left with the fact that the API is inconvenient: but we now have
the selectors module and can advocate that instead of breaking existing
code during a feature freeze period.

(or we can retry on EINTR, which has the benefit of not creating new
situations to deal with in existing code)

> The user now only has one thing to deal with instead of two: an empty
> list being returned; something they should already have been dealing
> with.

Returning an empty list when no timeout has been passed has never been a
feature of select(), which is why users are not expected to be dealing
with it.

I don't want this checked in to 3.4.

(Congratulations, this is my first "no" as a release manager!)

FYI Charles-François and me are working on a PEP to address this issue: the PEP 475. The PEP is not ready yet for a review.

See also Issue 23285 for the PEP

With PEP 475 now implemented (see Issue 23648), perhaps this could be closed? Or is there something else to be done?

> With PEP 475 now implemented (see Issue 23648), perhaps this could be closed? Or is there something else to be done?

Yes, this issue was fully fixed by the implementation of the PEP 475 in Python 3.5.