classification
Title: multiprocessing.util._eintr_retry doen't recalculate timeouts
Type: behavior Stage: resolved
Components: Versions: Python 3.3
process
Status: closed Resolution: out of date
Dependencies: Superseder:
Assigned To: Nosy List: pitrou, sbt, vstinner
Priority: normal Keywords: patch

Created on 2011-06-15 13:24 by sbt, last changed 2012-05-25 20:15 by sbt. This issue is now closed.

Files
File name Uploaded Description Edit
mp_remove_eintr_retry.patch sbt, 2012-03-16 14:22 review
Messages (8)
msg138364 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2011-06-15 13:24
multiprocessing.util._eintr_retry is only used to wrap select.select, but it fails to recalculate timeouts.

Also, it will never retry the function it wraps because of a missing "import errno".

I think it would be better to just implement the retrying version of select directly.
msg138674 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2011-06-19 23:29
subprocess._communicate_with_select() retries select.select() on EINTR: it recomputes timeout before each call.

while self._read_set or self._write_set:
    timeout = self._remaining_time(endtime)
    if timeout is not None and timeout < 0:
        raise TimeoutExpired(self.args, orig_timeout)
    try:
        (rlist, wlist, xlist) = \
            select.select(self._read_set, self._write_set, [],
                          timeout)
    except select.error as e:
        if e.args[0] == errno.EINTR:
            continue
        raise
    ...

It has a similar code for select.poll().

asyncore.poll() handles EINTR: it just returns.

> I think it would be better to just implement the retrying version
> of select directly.

It would be nice to share more code between subprocess and multiprocessing, but I don't know where such code should be moved. Create a new module just for one function is stupid. Handling EINTR is a common problem when managing subprocesses.

subprocess has a _eintr_retry_call() function.

multiprocessing has a @_eintr_retry decorator handling supporting more error types (use except (EnvironmentError, select.error):).
msg156035 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2012-03-16 14:22
_eintr_retry is currently unused.  The attached patch removes it.

If it is retained then we should at least add a warning that it does not recalculate timeouts.
msg156088 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-03-16 21:47
> _eintr_retry is currently unused.

AFAICT it's still used in forking.py.
msg157819 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2012-04-08 23:29
SocketServer has been changed to restart select() on EINTR, but it doesn't recompute the timeout.
msg157820 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2012-04-08 23:30
> SocketServer has been changed to restart select() on EINTR,
> but it doesn't recompute the timeout.

See issue #7978.
msg157821 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-04-08 23:32
Note that socketserver's timeout is not really important: it's just used for a polling loop, with a default of 0.5 seconds.
msg161619 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2012-05-25 20:15
_eintr_retry was removed by 99ef4501205b.
History
Date User Action Args
2012-05-25 20:15:26sbtsetstatus: open -> closed
type: behavior
messages: + msg161619

resolution: out of date
stage: resolved
2012-04-08 23:32:07pitrousetmessages: + msg157821
2012-04-08 23:30:21vstinnersetmessages: + msg157820
2012-04-08 23:29:56vstinnersetmessages: + msg157819
2012-03-16 21:47:17pitrousetmessages: + msg156088
versions: + Python 3.3
2012-03-16 14:22:55sbtsetfiles: + mp_remove_eintr_retry.patch
keywords: + patch
messages: + msg156035
2011-06-19 23:29:44vstinnersetmessages: + msg138674
2011-06-19 23:17:56vstinnersetnosy: + pitrou, vstinner
2011-06-15 13:24:28sbtcreate