Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

PEP 475: handle EINTR in the ssl module #68041

Closed
vstinner opened this issue Apr 2, 2015 · 5 comments
Closed

PEP 475: handle EINTR in the ssl module #68041

vstinner opened this issue Apr 2, 2015 · 5 comments

Comments

@vstinner
Copy link
Member

vstinner commented Apr 2, 2015

BPO 23853
Nosy @vstinner
Files
  • test_ssl_bug.patch
  • ssl_timeout.patch
    		•

    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-06.23:10:31.860>
created_at = <Date 2015-04-02.15:30:24.636>
labels = []
title = 'PEP 475: handle EINTR in the ssl module'
updated_at = <Date 2015-04-06.23:10:31.859>
user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2015-04-06.23:10:31.859>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2015-04-06.23:10:31.860>
closer = 'vstinner'
components = []
creation = <Date 2015-04-02.15:30:24.636>
creator = 'vstinner'
dependencies = []
files = ['38809', '38816']
hgrepos = []
issue_num = 23853
keywords = ['patch']
message_count = 5.0
messages = ['239924', '239927', '239967', '239982', '240182']
nosy_count = 2.0
nosy_names = ['vstinner', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue23853'
versions = ['Python 3.5']
    @vstinner
    Copy link
    Member Author

    vstinner commented Apr 2, 2015

    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 bpo-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.

    @vstinner
    Copy link
    Member Author

    vstinner commented Apr 2, 2015

    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).

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 3, 2015

    New changeset 753233baf27e by Victor Stinner in branch 'default':
    Issue bpo-23853: Cleanup _ssl.c
    https://hg.python.org/cpython/rev/753233baf27e

    @vstinner
    Copy link
    Member Author

    vstinner commented Apr 3, 2015

    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.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 6, 2015

    New changeset cdc83da0b0f8 by Victor Stinner in branch 'default':
    Issue bpo-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 bpo-23853: Methods of SSL socket don't reset the socket timeout anymore each
    https://hg.python.org/cpython/rev/5983f3fdacdb

    @vstinner vstinner closed this as completed Apr 6, 2015
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    None yet
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    1 participant
    @vstinner