Title: socket.readline() interface doesn't handle EINTR properly
Type: behavior Stage: test needed
Components: Extension Modules, Library (Lib) Versions: Python 3.0, Python 3.1, Python 2.7, Python 2.6
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: gregory.p.smith Nosy List: exarkun, gregory.p.smith, jorend, loewis, orenti, rhettg, sobomax, vila
Priority: normal Keywords: patch

Created on 2007-01-04 21:37 by sobomax, last changed 2010-12-15 20:00 by pitrou. This issue is now closed.

File name Uploaded Description Edit
diff sobomax, 2007-01-04 21:37 Proposed fix.
diff rhettg, 2009-08-02 22:05 Updated fix and test case
issue1628205-gps01.diff.txt gregory.p.smith, 2009-08-06 13:21
Messages (11)
msg51655 - (view) Author: Maxim Sobolev (sobomax) Date: 2007-01-04 21:37
The socket.readline() interface doesn't handle EINTR properly. Currently, when EINTR received exception is not handled and all data that has been in the buffer is lost. There is no way to recover that data from the code that uses the interface.

Correct behaviour would be to catch EINTR and restart recv(). Patch is attached.

Following is the real world example of how it affects httplib module:

  File "/usr/local/lib/python2.4/", line 1096, in __call__
    return self.__send(self.__name, args)
  File "/usr/local/lib/python2.4/", line 1383, in __request
  File "/usr/local/lib/python2.4/", line 1131, in request
    errcode, errmsg, headers = h.getreply()
  File "/usr/local/lib/python2.4/", line 1137, in getreply
    response = self._conn.getresponse()
  File "/usr/local/lib/python2.4/", line 866, in getresponse
  File "/usr/local/lib/python2.4/", line 336, in begin
    version, status, reason = self._read_status()
  File "/usr/local/lib/python2.4/", line 294, in _read_status
    line = self.fp.readline()
  File "/usr/local/lib/python2.4/", line 325, in readline
    data = recv(1)
error: (4, 'Interrupted system call')

msg51656 - (view) Author: Oren Tirosh (orenti) Date: 2007-01-07 18:24
You may have encountered this on sockets but *all* Python I/O does not handle restart on EINTR. 

The right place to fix this is probably in C, not the Python library. The places where an I/O operation could be interrupted are practically anywhere the GIL is released. This kind of change is likely to be controversial.
msg51657 - (view) Author: Maxim Sobolev (sobomax) Date: 2007-01-08 10:51
Well, it's not quite correct since for example tries to handle EINTR. The fundamental problem with socket.readline() is that it does internal buffering so that getting EINTR results in data being lost.

I don't think it has to be fixed in C, since recv() is very low-level interface and it is expected to return EINTR on signal, so that "fixing" it there could possibly break software that relies on this behaviour. And I don't quite buy your reasoning - "since it's broken in few more places let's keep it consistently broken everywhere". To me it sounds like attempt to hide the head in the sand instead of facing the problem at hand. Fixing socket.readline() may be the first step in improvind the library to handle this condition properly.
msg51658 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2007-02-16 13:05
I agree that this should be fixed; I'm not sure I like the proposed fixed, though. It discards the exception and keeps running.

What it (IMO) should do instead is abort, then return the data on the next invocation. Of course, this may have problems in itself, since the file descriptor might not report read-ready when passed to select or poll, even though data are available.

Please discuss this on python-dev (and elsewhere), and report what recommendations people made.
msg51659 - (view) Author: Jason Orendorff (jorend) Date: 2007-03-07 17:54
loewis: I think your idea is the right answer.  I'm not worried about select/poll.  Surely no one uses select/poll and socket._fileobject.readline() on the same socket.  select/poll are for nonblocking sockets; this readline() method doesn't even catch EWOULDBLOCK.

...In fact even if you did use select/poll on the (blocking) socket after readline() threw EINTR--which no one should do--I think it would still work just as expected unless you were doing something truly weird.
msg91209 - (view) Author: Rhett Garber (rhettg) Date: 2009-08-02 22:05
I've hit this issue as well.
Attached is an updated patch to 2.6 branch and a test case.
I wrote up more details here:
msg91359 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2009-08-06 13:21
nice test case rhettg.

This is a correctness issue to prevent data loss on EINTR.

I've attached a patch that builds on rhettg's but allows the EINTR signal 
to propagate upwards as desired by loweis and jorend for both read() and 
readline() calls.
msg91409 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2009-08-07 18:05
realistically, file objects (Objects/fileobject.c) never raise EINTR as 
they use the C library fread/fwrite/fclose underneath.  Why should a 
socket based file object every be allowed to raise EINTR rather than 
handling it internally?

IMHO people should only expect to handle EINTR when doing 
socket.send/recv or, not using a file-like object.
msg91414 - (view) Author: Rhett Garber (rhettg) Date: 2009-08-07 23:19
Good addition Gregory.
Totally agree on handling EINTR in even more cases.
You can't really expect the caller to know they need to deal with this
sort of thing when using these higher level call. The whole point is the
abstract away some of the complexity of dealing with the system call.
msg91532 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2009-08-13 18:57
fixed in trunk r74426.  socket.socket.sendall() and all
socket._fileobject methods (read/readline/write/flush) now properly
handle EINTR internally.

sendall will allow a python signal handler to raise an exception
aborting it in unknown state, otherwise it handles EINTR as it should
and continues sending.

I'm leaving this issue open until it is merged into py3k and considered
for 2.6 and/or 3.1 release backporting.
msg106334 - (view) Author: Jean-Paul Calderone (exarkun) * (Python committer) Date: 2010-05-23 15:22
Backported to release26-maint in r81488.
Date User Action Args
2011-06-05 20:31:39gregory.p.smithlinkissue3771 dependencies
2010-12-15 20:00:10pitrousetstatus: open -> closed
nosy: loewis, gregory.p.smith, jorend, orenti, exarkun, sobomax, vila, rhettg
resolution: fixed
versions: + Python 3.1, Python 2.7
2010-05-23 15:35:30vilasetnosy: + vila
2010-05-23 15:22:29exarkunsetnosy: + exarkun
messages: + msg106334
2009-08-13 18:57:21gregory.p.smithsetmessages: + msg91532
2009-08-07 23:19:34rhettgsetmessages: + msg91414
2009-08-07 18:05:47gregory.p.smithsetmessages: + msg91409
2009-08-06 13:21:33gregory.p.smithsetfiles: + issue1628205-gps01.diff.txt

nosy: + gregory.p.smith
messages: + msg91359

assignee: gregory.p.smith
2009-08-02 22:05:07rhettgsetfiles: + diff
nosy: + rhettg
messages: + msg91209

2009-03-30 18:48:20ajaksu2setstage: test needed
type: behavior
components: + Library (Lib)
versions: + Python 2.6, Python 3.0
2007-01-04 21:37:50sobomaxcreate