classification
Title: socket.create_connection() doesn't handle EINTR properly
Type: behavior Stage: test needed
Components: IO, Library (Lib) Versions: Python 3.4, Python 3.5, Python 2.7
process
Status: closed Resolution: duplicate
Dependencies: Superseder: Fix EINTR Socket Module issues in 2.7
View: 23863
Assigned To: gregory.p.smith Nosy List: flox, gregory.p.smith, martin.panter, meishao, neologix, pitrou, tholzer, vstinner
Priority: normal Keywords: patch

Created on 2014-02-12 15:13 by flox, last changed 2015-04-07 22:13 by gregory.p.smith. This issue is now closed.

Files
File name Uploaded Description Edit
httplib_2.6.4_eintr_patch.py tholzer, 2014-05-20 03:12 Python 2.6.4 httplib EINTR patch
httplib_2.7.3_eintr_patch.py tholzer, 2014-05-20 03:13 Python 2.7.3 httplib EINTR patch
socketmodule_2.7.6_eintr_patch.c tholzer, 2014-05-26 03:48 Python 2.7.6 socketmodule EINTR patch
socket_2.7.3_eintr_patch.py tholzer, 2014-05-29 22:16 python 2.7.3 socket.py EINTR patch
socket_2_7_2_patch.py meishao, 2014-05-30 01:03 python 2.7.2 socket.py EINTR patch
issue20611-connect-eintr-gps01.diff gregory.p.smith, 2014-06-03 07:44 review
Messages (20)
msg211095 - (view) Author: Florent Xicluna (flox) * (Python committer) Date: 2014-02-12 15:13
I had this sporadic traceback in a project:

  File "test.py", line 62, in <module>
    result = do_lqs(client, str(dnvn))
  File "test.py", line 25, in do_lqs
    qualif_service_id = client.create('ti.qualif.service', {})
  File "/srv/openerp/.buildout/eggs/ERPpeek-1.4.5-py2.6.egg/erppeek.py", line 894, in wrapper
    return self.execute(obj, method, *params, **kwargs)
  File "/srv/openerp/.buildout/eggs/ERPpeek-1.4.5-py2.6.egg/erppeek.py", line 636, in execute
    res = self._execute(obj, method, *params)
  File "/srv/openerp/.buildout/eggs/ERPpeek-1.4.5-py2.6.egg/erppeek.py", line 361, in <lambda>
    wrapper = lambda s, *args: s._dispatch(name, args)
  File "/usr/lib/python2.6/xmlrpclib.py", line 1489, in __request
    verbose=self.__verbose
  File "/usr/lib/python2.6/xmlrpclib.py", line 1235, in request
    self.send_content(h, request_body)
  File "/usr/lib/python2.6/xmlrpclib.py", line 1349, in send_content
    connection.endheaders()
  File "/usr/lib/python2.6/httplib.py", line 908, in endheaders
    self._send_output()
  File "/usr/lib/python2.6/httplib.py", line 780, in _send_output
    self.send(msg)
  File "/usr/lib/python2.6/httplib.py", line 739, in send
    self.connect()
  File "/usr/lib/python2.6/httplib.py", line 1112, in connect
    sock = socket.create_connection((self.host, self.port), self.timeout)
  File "/usr/lib/python2.6/socket.py", line 561, in create_connection
    raise error, msg
socket.error: [Errno 4] Interrupted system call



It seems that the EINTR should be caught by the standard library in all cases: http://bugs.python.org/issue1628205

But it's not the case for the "socket.create_connection" method (verified in 3.3 and 2.7 source code).
msg211128 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2014-02-13 05:13
> It seems that the EINTR should be caught by the standard library in all
cases:
> http://bugs.python.org/issue1628205

Yes, it should.

> But it's not the case for the "socket.create_connection" method (verified
in 3.3 and 2.7 source code).

It's not the case for *almost all* syscalls.
See http://bugs.python.org/issue18885
msg218831 - (view) Author: (tholzer) Date: 2014-05-20 03:12
We encountered the same problem, this is in the context of using PyQt (specifically QProcess) or twisted. They both rely on SIGCHLD for their notification framework.

I've attached a httplib EINTR patch for 2.6.4 & 2.7.3.
msg218833 - (view) Author: (tholzer) Date: 2014-05-20 04:09
Here is a reproducible test case:

import threading
import signal
import os
import httplib

def killer():
    while 1:
        os.kill(os.getpid(), signal.SIGINT)

def go():
    signal.signal(signal.SIGINT, lambda x,y: None)
    thread = threading.Thread(target=killer)
    thread.start()
    while 1:
        connection = httplib.HTTPConnection("localhost:80")
        connection.connect()
        connection.close()
        
if __name__ == '__main__':
    go()

Which gives:

Traceback (most recent call last):
  File "./repro1.py", line 22, in <module>
    go()
  File "./repro1.py", line 18, in go
    connection.connect()
  File ".../lib/python2.7/httplib.py", line 757, in connect
    self.timeout, self.source_address)
  File ".../lib/python2.7/socket.py", line 571, in create_connection
    raise err
socket.error:
msg218836 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2014-05-20 05:41
As said in a previous comment, we don't want to have EINTR handling
code everywhere.
The right way to do this is to handle it at the syscall level.
msg218841 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-05-20 11:39
@tholzer, to clarify Charles-François's comment: EINTR should be handled in socket.connect() and socket.getaddrinfo() (the two syscalls called by create_connection(), AFAIR).
msg219121 - (view) Author: (tholzer) Date: 2014-05-26 02:58
No problem, I've attached a patch for socket.py for Python 2.7.3.

A few notes:

getaddrinfo (and gethostbyname, etc.) are already immune to this bug, so I've just fixed the connect() call.

The socket does need to be closed after EINTR, otherwise a EINPROGRESS might get returned on subsequent connect() calls.
msg219122 - (view) Author: (tholzer) Date: 2014-05-26 03:42
I've also attached a potential patch for the C module Modules/socketmodule.c inside internal_connect().

A few notes:

This seems to work both without time-out and with time-out sockets (non-blocking).

One concern would be a signal storm prolonging the operation beyond the time-out. Should we keep track of the actual time taken in this loop and check it against the 'timeout' parameter ?

Also, I don't think we can call PyErr_CheckSignals() in this context. Does this need to happen at all ?
msg219337 - (view) Author: Syou Ei (meishao) * Date: 2014-05-29 06:02
http://bugs.python.org/issue21602

The smtplib.py also has the same problem.
The EINTR cannot be handled properly.
msg219340 - (view) Author: Syou Ei (meishao) * Date: 2014-05-29 08:30
@neologix,
May I attach the patch file of smtplib.py for review?
msg219343 - (view) Author: Florent Xicluna (flox) * (Python committer) Date: 2014-05-29 09:26
@meishao

Previous comments answer your question :
http://bugs.python.org/issue20611#msg218836
http://bugs.python.org/issue20611#msg218841
msg219344 - (view) Author: Syou Ei (meishao) * Date: 2014-05-29 10:18
@flox

Thank you for your comment.
So we just only modify the socket.py to handle the system level call, is it right?
Please let me attach the patch file of socket.py for 2.7.2.
msg219371 - (view) Author: (tholzer) Date: 2014-05-29 22:16
Oops, I missed a break statement at the end of socket_2.7.3_eintr_patch.py. I've fixed this now in the attached patch.

@meishao

Could you please also update your socket_2_7_2_patch.py and add the missing break statement ?
msg219375 - (view) Author: Syou Ei (meishao) * Date: 2014-05-30 01:03
@tholzer

I've updated socket_2_7_2_patch.py and added the missing break statement.
msg219376 - (view) Author: (tholzer) Date: 2014-05-30 01:37
And a test case for smtplib:

import threading
import signal
import os
import smtplib

def go():
    running = True
    pid = os.getpid()

    def killer():
        while running: 
            os.kill(pid, signal.SIGINT)

    signal.signal(signal.SIGINT, lambda x,y: None)
    thread = threading.Thread(target=killer)
    thread.start()

    while 1:
        try:
            smtplib.SMTP('localhost')
        except Exception, ex:
            running = False
            raise
        
if __name__ == '__main__':
    go()

Fails with:

socket.error: [Errno 4] Interrupted system call
msg219668 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2014-06-03 07:44
Something like the patch i'm attaching to socketmodule.c is what I would prefer.  I haven't looked at or tried tests for it yet.
msg223890 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-07-24 20:30
The issue is just an example of the main issue #18885 which proposes to retry interrupted syscalls. I hesitate to mark it as duplicate, but it contains an interesting patch.
msg235544 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-02-08 03:38
See also PEP 475 and Issue 23285 for the general fix in Python 3
msg240194 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-04-06 23:53
This issue was fixed in Python 3.5 as part of the PEP 475: see issue #23618 which modified socket.socket.connect() to handle EINTR. It's not as simple as retrying connect() in a loop. You must respect the socket timeout (it's better to have a monotonic clock here, it's now always the case in Python 3.5). When connect() returns EINTR, the connection runs asynchronously, you have to call select() to wait until the connection completes or fails. Then you have to call getsockopt() to get the socket error code.

In Python 3.5, socket.socket.connect() still raises InterruptedError if the socket is non-blocking:
https://docs.python.org/dev/library/socket.html#socket.socket.connect

issue20611-connect-eintr-gps01.diff calls again connect() if connect() failed with EINTR. According to the issue #23618, it might work on some platforms, but it's not portable.

For Python 2.7 and 3.4, instead of fixing socket.socket.connect(), which requires complex code, we may only workaround the issue in create_connection(). If connect() raises OSError(EINTR), drop the socket and retry with a fresh connection in a loop until the connection completes or raises a different exception. (And do that for each address.)
msg240232 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2015-04-07 22:13
i'm moving this to the more recent issue as i like the patch in that one better.
History
Date User Action Args
2015-04-07 22:13:49gregory.p.smithsetstatus: open -> closed
resolution: duplicate
2015-04-07 22:13:36gregory.p.smithsetsuperseder: Fix EINTR Socket Module issues in 2.7
messages: + msg240232
2015-04-06 23:53:42vstinnersetmessages: + msg240194
2015-02-08 03:38:18martin.pantersetnosy: + martin.panter
messages: + msg235544
2014-07-24 20:30:58vstinnersetmessages: + msg223890
2014-06-03 07:45:05gregory.p.smithsetversions: + Python 3.5, - Python 3.3
2014-06-03 07:44:56gregory.p.smithsetstage: test needed
2014-06-03 07:44:46gregory.p.smithsetfiles: + issue20611-connect-eintr-gps01.diff
assignee: gregory.p.smith
messages: + msg219668

keywords: + patch
2014-05-30 01:37:46tholzersetmessages: + msg219376
2014-05-30 01:03:16meishaosetfiles: + socket_2_7_2_patch.py

messages: + msg219375
2014-05-30 01:01:28meishaosetfiles: - socket_2_7_2_patch.py
2014-05-29 22:16:55tholzersetfiles: - socket_2.7.3_eintr_patch.py
2014-05-29 22:16:11tholzersetfiles: + socket_2.7.3_eintr_patch.py

messages: + msg219371
2014-05-29 10:18:07meishaosetfiles: + socket_2_7_2_patch.py

messages: + msg219344
2014-05-29 09:26:53floxsetmessages: + msg219343
2014-05-29 08:30:17meishaosetmessages: + msg219340
2014-05-29 07:43:29neologixlinkissue21602 superseder
2014-05-29 06:02:16meishaosetnosy: + meishao
messages: + msg219337
2014-05-26 03:49:15tholzersetfiles: - socketmodule_2.7.6_eintr_patch.c
2014-05-26 03:48:52tholzersetfiles: + socketmodule_2.7.6_eintr_patch.c
2014-05-26 03:42:02tholzersetfiles: + socketmodule_2.7.6_eintr_patch.c

messages: + msg219122
2014-05-26 02:58:56tholzersetfiles: + socket_2.7.3_eintr_patch.py

messages: + msg219121
2014-05-20 11:39:48pitrousetmessages: + msg218841
2014-05-20 05:41:32neologixsetmessages: + msg218836
2014-05-20 04:09:06tholzersetmessages: + msg218833
2014-05-20 03:13:37tholzersetfiles: + httplib_2.7.3_eintr_patch.py
2014-05-20 03:13:01tholzersetfiles: + httplib_2.6.4_eintr_patch.py
nosy: + tholzer
messages: + msg218831

2014-02-13 05:13:36neologixsetmessages: + msg211128
2014-02-12 15:19:07floxsetnosy: + gregory.p.smith, pitrou
2014-02-12 15:16:07vstinnersetnosy: + vstinner, neologix
2014-02-12 15:13:52floxcreate