classification
Title: PEP 475: handle EINTR in the ssl module
Type: Stage:
Components: Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: python-dev, vstinner
Priority: normal Keywords: patch

Created on 2015-04-02 15:30 by vstinner, last changed 2015-04-06 23:10 by vstinner. This issue is now closed.

Files
File name Uploaded Description Edit
test_ssl_bug.patch vstinner, 2015-04-02 15:33 review
ssl_timeout.patch vstinner, 2015-04-03 12:17 review
Messages (5)
msg239924 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-04-02 15:30
The _ssl module uses an helper function check_socket_and_wait_for_timeout() to poll until the socket is ready (got data or became writable). check_socket_and_wait_for_timeout() always uses the socket timeout. If select() or poll() is interrupted by a signal (fails with EINTR), check_socket_and_wait_for_timeout() is restarted with the same timeout, which doesn't respect the contract of the timeout: the operation must timeout if it takes more than timeout seconds.

The code must be modified to recompute the timeout, as done in the new sock_call() function of Modules/socketmodule.c (issue #23618). At least, the timeout must decreases when select()/poll() fails with EINTR.

Currently, the timeout is reset after each read/write/handshake operation. IMO the timeout must apply on the total duration of the ssl method, not be reset. But changing this may break backward compatibility :-/

Note: if the signal handler raises an exception, the ssl method fails with the exception. This issue is specific to signal handlers not raising an exception.
msg239927 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-04-02 15:33
test_ssl_bug.patch: Modify test_handshake_timeout() of test_ssl to show the bug: test_handshake_timeout() hangs with the patch (which sends a signal every millisecond).
msg239967 - (view) Author: Roundup Robot (python-dev) Date: 2015-04-03 11:36
New changeset 753233baf27e by Victor Stinner in branch 'default':
Issue #23853: Cleanup _ssl.c
https://hg.python.org/cpython/rev/753233baf27e
msg239982 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-04-03 12:17
Here is a patch to fix the issue: recompute the timeout.

It's unclear to me if we should reset the timeout after each successful read/write, or if the timeout is "global" (total duration of the ssl method). I asked the question on the python-dev mailing list.
https://mail.python.org/pipermail/python-dev/2015-April/139001.html

My patch uses a global timeout (never reset the timeout), whereas socket.sendall() resets the timeout at each send() success.
msg240182 - (view) Author: Roundup Robot (python-dev) Date: 2015-04-06 21:25
New changeset cdc83da0b0f8 by Victor Stinner in branch 'default':
Issue #23853: socket.socket.sendall() does no more reset the socket timeout
https://hg.python.org/cpython/rev/cdc83da0b0f8

New changeset 5983f3fdacdb by Victor Stinner in branch 'default':
Issue #23853: Methods of SSL socket don't reset the socket timeout anymore each
https://hg.python.org/cpython/rev/5983f3fdacdb
History
Date User Action Args
2015-04-06 23:10:31vstinnersetstatus: open -> closed
resolution: fixed
2015-04-06 21:25:12python-devsetmessages: + msg240182
2015-04-03 12:17:25vstinnersetfiles: + ssl_timeout.patch

messages: + msg239982
2015-04-03 11:37:00python-devsetnosy: + python-dev
messages: + msg239967
2015-04-02 15:33:14vstinnersetfiles: + test_ssl_bug.patch
keywords: + patch
messages: + msg239927
2015-04-02 15:30:56vstinnerlinkissue23648 dependencies
2015-04-02 15:30:24vstinnercreate