Skip to content

PEP 475 - EINTR handling #67474

Closed
Closed
PEP 475 - EINTR handling#67474
Labels
stdlibStandard Library Python modules in the Lib/ directorytype-featureA feature request or enhancement

Description

@neologix
neologix
mannequin
opened on Jan 20, 2015
BPO 23285
Nosy @pitrou, @vstinner, @ned-deily, @vadmium
Files
  • ff1274594739.diff
  • eintr.diff
  • eintr-1.diff
  • eintr-2.diff
  • eintr-3.diff
  • eintr-4.diff
  • fileio.patch
  • setitimer.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-03-20.11:44:35.815>
created_at = <Date 2015-01-20.23:03:28.161>
labels = ['type-feature', 'library']
title = 'PEP 475 - EINTR handling'
updated_at = <Date 2016-07-10.09:23:45.050>
user = 'https://bugs.python.org/neologix'

    bugs.python.org fields:

    activity = <Date 2016-07-10.09:23:45.050>
actor = 'berker.peksag'
assignee = 'none'
closed = True
closed_date = <Date 2015-03-20.11:44:35.815>
closer = 'vstinner'
components = ['Library (Lib)']
creation = <Date 2015-01-20.23:03:28.161>
creator = 'neologix'
dependencies = []
files = ['37797', '37802', '37905', '37909', '37910', '37935', '38309', '38403']
hgrepos = ['293']
issue_num = 23285
keywords = ['patch']
message_count = 33.0
messages = ['234400', '234402', '234403', '234412', '234425', '234874', '234879', '234931', '234950', '234952', '234953', '234955', '234989', '234993', '234997', '235010', '235091', '235092', '235285', '235424', '235435', '235451', '235525', '235546', '237100', '237101', '237159', '237193', '237194', '237611', '237612', '237613', '238650']
nosy_count = 6.0
nosy_names = ['pitrou', 'vstinner', 'ned.deily', 'neologix', 'python-dev', 'martin.panter']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'patch review'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue23285'
versions = ['Python 3.5']

    Activity

    1. neologix commented on Jan 29, 2015

      @neologix
      neologixmannequin
      on Jan 29, 2015
      MannequinAuthor

      eintr-1.diff doesn't seem to make any significant difference from eintr.diff on my system. It's still pegging a CPU at 100% and takes 7 minutes wall time to complete.

      Alright, enough played: the patch attached uses a memoryview and
      socket.recv_into() to remove all memory allocations: if this doesn't
      fix your problem, I don't know what's going on...

    2. ned-deily commented on Jan 29, 2015

      @ned-deily
      ned-deily
      on Jan 29, 2015
      Member

      With eintr-2.diff, fast!:

      $ time ./python ~/Projects/PyDev/active/dev/3x/source/Lib/test/eintrdata/eintr_tester.py
test_read (__main__.OSEINTRTest) ... ok
test_wait (__main__.OSEINTRTest) ... ok
test_wait3 (__main__.OSEINTRTest) ... ok
test_wait4 (__main__.OSEINTRTest) ... ok
test_waitpid (__main__.OSEINTRTest) ... ok
test_write (__main__.OSEINTRTest) ... ok
test_accept (__main__.SocketEINTRTest) ... ok
test_recv (__main__.SocketEINTRTest) ... ok
test_recvmsg (__main__.SocketEINTRTest) ... ok
test_send (__main__.SocketEINTRTest) ... ok
test_sendall (__main__.SocketEINTRTest) ... ok
test_sendmsg (__main__.SocketEINTRTest) ... ok

      Ran 12 tests in 7.652s

      OK

      real 0m7.959s
      user 0m2.573s
      sys 0m1.604s

      Instrumented test_send, 3 socket.send calls, many socket.recv_into calls:

      test_send (main.SocketEINTRTest) ... rd SO_RCVBUF default was 8192, wr SO_SNDBUF default was 8192
      len(data) = 16777215

      sent = 8192, total written = 8192
      sent = 4440064, total written = 4448256
      sent = 1232895, total written = 16777215

      received = 8192, total read = 8192
      received = 8192, total read = 16384
      received = 8192, total read = 24576
      [...]
      received = 8192, total read = 16760832
      received = 8192, total read = 16769024
      received = 8191, total read = 16777215

    3. neologix commented on Jan 29, 2015

      @neologix
      neologixmannequin
      on Jan 29, 2015
      MannequinAuthor

      With eintr-2.diff, fast!:

      Victory \°/.

      Instrumented test_send, 3 socket.send calls, many socket.recv_into calls:

      Yep, that's expected.
      I think we should keep the default socket buffer size: it increases
      the test coverage, and it's probably not worth trying to increase it
      to make the test a bit faster, especially since it spends so much time
      sleeping.

      Antoine, I'm now happy with the patch, so we'll be waiting for your
      decision on the PEP with Victor :-).

    4. pitrou commented on Jan 30, 2015

      @pitrou
      pitrou
      on Jan 30, 2015
      Member

      Victor, do you think there's anything left to do?

    5. neologix commented on Jan 31, 2015

      @neologix
      neologixmannequin
      on Jan 31, 2015
      MannequinAuthor

      I just realized I didn't retry upon EINTR for open(): eintr-4.diff
      adds this, along with tests (using a fifo).

      Also, I added comments explaining why we don't retry upon close() (see
      e.g. http://lwn.net/Articles/576478/ and
      http://linux.derkeiler.com/Mailing-Lists/Kernel/2005-09/3000.html).

    6. vstinner commented on Jan 31, 2015

      @vstinner
      vstinner
      on Jan 31, 2015
      Member

      Charles-François Natali added the comment:

      I just realized I didn't retry upon EINTR for open(): eintr-4.diff
      adds this, along with tests (using a fifo).

      Also, I added comments explaining why we don't retry upon close() (see
      e.g. http://lwn.net/Articles/576478/ and
      http://linux.derkeiler.com/Mailing-Lists/Kernel/2005-09/3000.html).

      I didn't read these articles yet, but I will. IMO the PEP must be
      updated if we don't retry in close(), since close() is used as an
      example where "EINTR is unexpected"!

    7. pitrou commented on Feb 2, 2015

      @pitrou
      pitrou
      on Feb 2, 2015
      Member

      PEP is now updated.

    8. vstinner commented on Feb 5, 2015

      @vstinner
      vstinner
      on Feb 5, 2015
      Member

      Would it be possible to push the latest patch right now, and fix remaining issues (if there are known issues?), before Python 3.4 alpha 1? According to the PEP-478, the alpha 1 is scheduled for this sunday (February 8, 2015).

    9. pitrou commented on Feb 5, 2015

      @pitrou
      pitrou
      on Feb 5, 2015
      Member

      Would it be possible to push the latest patch right now

      It's ok for me. Please watch the buildbots :)

    10. neologix commented on Feb 5, 2015

      @neologix
      neologixmannequin
      on Feb 5, 2015
      MannequinAuthor

      Antoine Pitrou added the comment:

      > Would it be possible to push the latest patch right now

      It's ok for me. Please watch the buildbots :)

      Cool, I'll push on Friday evening or Saturday.

    11. python-dev commented on Feb 7, 2015

      @python-dev
      python-devmannequin
      on Feb 7, 2015
      Mannequin

      New changeset 5b63010be19e by Charles-François Natali in branch 'default':
      Issue bpo-23285: PEP-475 -- Retry system calls failing with EINTR.
      https://hg.python.org/cpython/rev/5b63010be19e

    12. python-dev commented on Feb 8, 2015

      @python-dev
      python-devmannequin
      on Feb 8, 2015
      Mannequin

      New changeset 000bbdf0ea76 by Ned Deily in branch 'default':
      Issue bpo-23285: Install new test directory.
      https://hg.python.org/cpython/rev/000bbdf0ea76

    13. vstinner commented on Mar 3, 2015

      @vstinner
      vstinner
      on Mar 3, 2015
      Member

      The change on Modules/_io/fileio.c is wrong: functions may return None with an exception set. It is wrong because a function must return a result with no exception set, or NULL and an exception set.

      Attached patch fixes this issue.

    14. vstinner commented on Mar 3, 2015

      @vstinner
      vstinner
      on Mar 3, 2015
      Member

      Note: I found the bug while working on a patch for bpo-22181.

      My test is this shell script:

      $ while true; do ./python -c 'import os, signal; signal.setitimer(signal.ITIMER_REAL, 0.001, 0.0001); signal.signal(signal.SIGALRM, lambda *args: print(".", end="")); print("urandom"); x=os.urandom(5000); print("ok"); signal.alarm(0)'; if [ $? -ne 142 -a $? -ne 0 ]; then break; fi done

      The test calls print() in a signal handler which can likely create a reentrant call, which is forbidden. But Python handles this case, it's fine.

      After removing all debug prints and reverting the fix on fileio.c, the test doesn't crash with the assertion error anymore. Before, an assertion failed because fileio_write() returned Py_None with an exception set.

    15. neologix commented on Mar 3, 2015

      @neologix
      neologixmannequin
      on Mar 3, 2015
      MannequinAuthor

      @victor: please commit.
      Would be nice to have a test for it;

    16. vstinner commented on Mar 4, 2015

      @vstinner
      vstinner
      on Mar 4, 2015
      Member

      Would be nice to have a test for it;

      There are already some tests on EINTR in test_file_eintr and test_subprocess (test_communicate_eintr()).

    17. python-dev commented on Mar 4, 2015

      @python-dev
      python-devmannequin
      on Mar 4, 2015
      Mannequin

      New changeset cad6eac598ec by Victor Stinner in branch 'default':
      Issue bpo-23285: Fix handling of EINTR in fileio.c
      https://hg.python.org/cpython/rev/cad6eac598ec

    18. vstinner commented on Mar 9, 2015

      @vstinner
      vstinner
      on Mar 9, 2015
      Member

      test_setitimer.patch: hack unittest.TestCase.run() to inject signals every 1 ms, send the first signal in 100 ms.

      This patch helped me to find that socket.connect() doesn't handle EINTR yet: see issue bpo-23618 "PEP-475: handle EINTR in the socket module".

    19. pitrou commented on Mar 9, 2015

      @pitrou
      pitrou
      on Mar 9, 2015
      Member

      For the record, it seems test_eintr sometimes left zombie processes in my machine where I run reference leak tests every night. I didn't investigate and just disabled the tests.

    20. pitrou commented on Mar 9, 2015

      @pitrou
      pitrou
      on Mar 9, 2015
      Member

      (I'll add that that machine is hosted on an OpenVZ-based VPS, so perhaps there are issues with the old patched kernel and whatnot?)

    21. vstinner commented on Mar 20, 2015

      @vstinner
      vstinner
      on Mar 20, 2015
      Member

      For the record, it seems test_eintr sometimes left zombie processes in my machine where I run reference leak tests every night. I didn't investigate and just disabled the tests.

      (I'll add that that machine is hosted on an OpenVZ-based VPS, so perhaps there are issues with the old patched kernel and whatnot?)

      On my Fedora 21 (on a physical PC, not virtualized), I ran "./python -m test -R 3:3: test_eintr" 3 times. After that, I didn't see any zombi Python process. If I cannot reproduce the issue, I cannot fix it. I bet that it's related to OpenVZ.

      IMO this issue can be closed. It already has a long history. I prefer to open new issues. See the issue bpo-23648 "PEP-475 meta issue" which lists all issues related to the PEP-475. I opened new issues for each module which didn't handle completly the PEP-475 yet.

      By the way: great job Charles-François! I like your changeset, it works well.

    22. vstinner
      closed this as completedon Mar 20, 2015
    23. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      stdlibStandard Library Python modules in the Lib/ directorytype-featureA feature request or enhancement

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions