PEP 475: handle EINTR in the socket module (connect)

[vstinner]

BPO	23618
Nosy	@pitrou, @vstinner, @serhiy-storchaka
Files	connect_eintr.patch connect_eintr.py connect_eintr-2.patch connect_test.c connect_eintr-3.patch connect_eintr-4.patch connect_eintr-4.patch test_connect_eintr.py test_connect_eintr2.py test_connect_eintr3.py

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2015-04-02.14:17:55.254>
created_at = <Date 2015-03-09.09:08:33.169>
labels = ['extension-modules']
title = 'PEP 475: handle EINTR in the socket module (connect)'
updated_at = <Date 2015-04-09.08:32:58.616>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2015-04-09.08:32:58.616>
actor = 'python-dev'
assignee = 'none'
closed = True
closed_date = <Date 2015-04-02.14:17:55.254>
closer = 'vstinner'
components = ['Extension Modules']
creation = <Date 2015-03-09.09:08:33.169>
creator = 'vstinner'
dependencies = []
files = ['38402', '38408', '38409', '38411', '38412', '38766', '38767', '38803', '38805', '38806']
hgrepos = []
issue_num = 23618
keywords = ['patch']
message_count = 31.0
messages = ['237609', '237610', '237616', '237619', '237656', '237657', '237666', '237669', '238624', '238720', '239444', '239640', '239690', '239693', '239719', '239741', '239744', '239746', '239748', '239775', '239896', '239899', '239902', '239903', '239904', '239905', '239906', '239911', '239912', '239913', '240312']
nosy_count = 5.0
nosy_names = ['pitrou', 'vstinner', 'neologix', 'python-dev', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue23618'
versions = ['Python 3.5']

[vstinner]

The PEP-475 has been accepted and is partialy implemented. The socket module is not fully patched to handle EINTR. For example, socket.socket.connect() doesn't handle EINTR yet.

Attached patch fixes socket.connect() to handle EINTR. It should fix issue bpo-11266 and bpo-20611 in Python 3.5 (Python 2.7 and 3.4 will need to be patched to handle explicitly InterruptedError/OSError(EINTR) in Python).

By the way, some socket functions handle EINTR but don't recompute the timeout yet. _PyTime_monotonic() should be used.

[neologix]

If EINTR is received during connect, the socket is unusable, that's why i
didn't implement it.

[vstinner]

If EINTR is received during connect, the socket is unusable,
that's why i didn't implement it.

Can you elaborate?

socket.connect() should be modified according to the PEP-475:
https://www.python.org/dev/peps/pep-0475/#modified-functions

What do you mean by "unusable"? Is it possible to retry connect()? Is it safe to call getsockopt(fd, SOL_SOCKET, SO_ERROR) until it returns EISCONN?

For high-level functions for socket.create_connection(), how should we handle InterruptedError?

See also this change in asyncio to handle InterruptedError in asyncio:
https://hg.python.org/cpython/rev/ad67f66a5f3c
(I wrote it and I didn't test my change, I didn't know how to test it.)
Tulip issue: https://code.google.com/p/tulip/issues/detail?id=205

[pitrou]

About EINTR and connect(), I've found the following insightful page:
http://www.madore.org/~david/computers/connect-intr.html

Official POSIX wording is this:

"""If connect() is interrupted by a signal that is caught while blocked waiting to establish a connection, connect() shall fail and set errno to [EINTR], but the connection request shall not be aborted, and the connection shall be established asynchronously."""

[vstinner]

connect_eintr.py: script calling socket.connect() in a loop and sending SIGARLM signal every millisecond.

[vstinner]

Oops, connect_eintr.py noticed me (thanks to my recent change of the issue bpo-23571 !) that connect_eintr.patch is wrong: socket.connect() returned None with an exception sent, send connect() was interrupted by SIGINT (CTRL+c).

Fixed patch.

[vstinner]

http://www.madore.org/~david/computers/connect-intr.html

This article contains a program connect_test.c to test how connect() behaves on EINTR. Since it's in the public domain, I attached a copy.

The program contains the comment: "All systems function as expected when TEST_TWO is set."

If TEST_TWO is defined, poll() is used to wait until the socket is writable, and then getsockopt(SO_ERROR) is used to check if the error is now zero, when connect() fails with EINTR.

[vstinner]

connect_eintr-3.patch: Different patch, don't retry connect() if it returns EINTR, but poll using poll/select. The patch changes also the asyncio module.

[vstinner]

I tested polling+getsockopt(SO_ERROR) with connect_test.c on Linux, it works:

haypo@smithers$ gcc -D TEST_TWO=1 connect_test.c -o connect_test
haypo@smithers$ ./connect_test
Will try to connect to 127.0.0.1 on port 80
(connect has been interrupted and now completed successfully)

[neologix]

Could you regenerate your latest patch?
It doesn't show properly in the review tool.

Also, what's with the assert?

[python-dev]

New changeset f841d3bc30ee by Victor Stinner in branch 'default':
Issue bpo-23618, bpo-22117: refactor socketmodule.c
https://hg.python.org/cpython/rev/f841d3bc30ee

[vstinner]

test_selectors.patch: Enhance test_selector to test the two kinds of signal handlers: one raises an exception, the other one doesn't. I wait until kqueue & devpoll retry on EINTR to push test_selectors.patch.

[python-dev]

New changeset e8246baad0f6 by Victor Stinner in branch 'default':
Issue bpo-23618: Refactor the _socket module
https://hg.python.org/cpython/rev/e8246baad0f6

New changeset fa5542660b17 by Victor Stinner in branch 'default':
Issue bpo-23618: Refactor internal_select() to prepare socket.connect() for EINTR
https://hg.python.org/cpython/rev/fa5542660b17

New changeset c027d6468667 by Victor Stinner in branch 'default':
Issue bpo-23618: internal_connect_select() now waits also for error events
https://hg.python.org/cpython/rev/c027d6468667