Skip to content

subprocess module does not check WIFSTOPPED on SIGCHLD #73521

Closed
Closed
subprocess module does not check WIFSTOPPED on SIGCHLD#73521
Assignees
gpshead
Labels
stdlibStandard Library Python modules in the Lib/ directorytype-bugAn unexpected behavior, bug, or error

Description

@ZachRiggle
ZachRiggle
mannequin
opened on Jan 20, 2017
BPO 29335
Nosy @gpshead, @vstinner, @ned-deily
PRs
  • bpo-29335 - apply suggested test_subprocess unittest simplifications. #1757
  • bpo-30764: test_subprocess uses SuppressCrashReport #2405
  • [3.6] bpo-30764: test_subprocess uses SuppressCrashReport (#2405) #2410
  • [3.5] bpo-30764: test_subprocess uses SuppressCrashReport (#2405) #2411
    		•
    Files
  • bug.py: Example script hitting "Should never happen"
  • issue29335-gps01.diff
  • issue29335-gps02.diff
    		•

    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 = 'https://github.com/gpshead'
closed_at = <Date 2017-05-24.01:36:09.178>
created_at = <Date 2017-01-20.20:48:08.437>
labels = ['type-bug', 'library']
title = 'subprocess module does not check WIFSTOPPED on SIGCHLD'
updated_at = <Date 2017-06-26.22:00:53.909>
user = 'https://bugs.python.org/ZachRiggle'

    bugs.python.org fields:

    activity = <Date 2017-06-26.22:00:53.909>
actor = 'vstinner'
assignee = 'gregory.p.smith'
closed = True
closed_date = <Date 2017-05-24.01:36:09.178>
closer = 'gregory.p.smith'
components = ['Library (Lib)']
creation = <Date 2017-01-20.20:48:08.437>
creator = 'Zach Riggle'
dependencies = []
files = ['46363', '46385', '46386']
hgrepos = []
issue_num = 29335
keywords = ['patch']
message_count = 17.0
messages = ['285921', '285996', '286023', '286031', '286036', '286046', '286050', '286057', '286099', '286100', '286102', '294251', '294308', '296907', '296910', '296957', '296959']
nosy_count = 5.0
nosy_names = ['gregory.p.smith', 'vstinner', 'ned.deily', 'python-dev', 'Zach Riggle']
pr_nums = ['1757', '2405', '2410', '2411']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue29335'
versions = ['Python 2.7', 'Python 3.5', 'Python 3.6']

    Activity

    1. ZachRiggle commented on Jan 20, 2017

      @ZachRiggle
      ZachRigglemannequin
      on Jan 20, 2017
      MannequinAuthor

      The attached script hits some "This should never happen" code in the subprocess module.

      These lines here:
      https://github.com/python/cpython/blob/2.7/Lib/subprocess.py#L1036-L1038

      The root cause is a lack of checking WIFSTOPPED and WSTOPSIG in the handler.

      When a process elects into being ptraced via PTRACE_TRACEME, it is stopped on the SIGSEGV instead of terminating, allowing the user to attach a debugger before the kernel destroys the process.

      This bug makes it impossible to wait on any process which crashes, which is set up to wait for a debugger.

    2. ZachRiggle
      added
      stdlibStandard Library Python modules in the Lib/ directory
      on Jan 20, 2017

    3. ZachRiggle commented on Jan 22, 2017

      @ZachRiggle
      ZachRigglemannequin
      on Jan 22, 2017
      MannequinAuthor

      To further clarify the report:

      When the attached proof-of-concept is executed, a RuntimeException is raised, which has a comment "Should never happen".

      The issue isn't due to SIGCHLD, but rather following a waitpid() call. The code attempts to suss the exit code / reason for waitpid() returning, but does not check for WIFSTOPPED in its handler.

    4. gpshead
      self-assigned this
      on Jan 22, 2017
    5. gpshead
      changed the title [-]Python 2.7 subprocess module does not check WIFSTOPPED on SIGCHLD[/-] [+]subprocess module does not check WIFSTOPPED on SIGCHLD[/+] on Jan 22, 2017

    6. gpshead commented on Jan 22, 2017

      @gpshead
      gpshead
      on Jan 22, 2017
      Member

      The attached patch should fix it.

      I want to incorporate a bug.py like regression test into test_subprocess.py.

    7. gpshead commented on Jan 22, 2017

      @gpshead
      gpshead
      on Jan 22, 2017
      Member

      test added.

    8. gpshead
      added
      type-bugAn unexpected behavior, bug, or error
      on Jan 22, 2017

    9. python-dev commented on Jan 23, 2017

      @python-dev
      python-devmannequin
      on Jan 23, 2017
      Mannequin

      New changeset 269296b2a047 by Gregory P. Smith in branch '3.5':
      Issue bpo-29335: Fix subprocess.Popen.wait() when the child process has
      https://hg.python.org/cpython/rev/269296b2a047

      New changeset ed5255a61648 by Gregory P. Smith in branch '3.6':
      Issue bpo-29335: Fix subprocess.Popen.wait() when the child process has
      https://hg.python.org/cpython/rev/ed5255a61648

      New changeset 4f5e7d018195 by Gregory P. Smith in branch 'default':
      Issue bpo-29335: Fix subprocess.Popen.wait() when the child process has
      https://hg.python.org/cpython/rev/4f5e7d018195

    10. ned-deily commented on Jan 23, 2017

      @ned-deily
      ned-deily
      on Jan 23, 2017
      Member

      Among other buildbot failures:

      http://buildbot.python.org/all/builders/x86%20Tiger%203.6/builds/142/steps/test/logs/stdio

      ======================================================================
      ERROR: test_child_terminated_in_stopped_state (test.test_subprocess.POSIXProcessTestCase)
      Test wait() behavior when waitpid returns WIFSTOPPED; bpo-29335.
      ----------------------------------------------------------------------

      Traceback (most recent call last):
  File "/Users/db3l/buildarea/3.6.bolen-tiger/build/Lib/test/test_subprocess.py", line 2514, in test_child_terminated_in_stopped_state
    libc = ctypes.CDLL(libc_name)
  File "/Users/db3l/buildarea/3.6.bolen-tiger/build/Lib/ctypes/__init__.py", line 348, in __init__
    self._handle = _dlopen(self._name, mode)
OSError: dlopen(libc..dylib, 6): image not found

      Ran 260 tests in 102.297s

      Also, http://buildbot.python.org/all/builders/x86%20Ubuntu%20Shared%203.x/builds/240/steps/test/logs/stdio

    11. python-dev commented on Jan 23, 2017

      @python-dev
      python-devmannequin
      on Jan 23, 2017
      Mannequin

      New changeset 8e3d412f8e89 by Gregory P. Smith in branch '2.7':
      Issue bpo-29335: Fix subprocess.Popen.wait() when the child process has
      https://hg.python.org/cpython/rev/8e3d412f8e89

    12. gpshead commented on Jan 23, 2017

      @gpshead
      gpshead
      on Jan 23, 2017
      Member

      thanks Ned, I was awaiting interesting buildbot results. :)

      fixed in 2.7 and 3.5 onwards. thanks for the report Zach.

      not closing until I also apply the fix to the subprocess32 backport.

    13. ZachRiggle commented on Jan 23, 2017

      @ZachRiggle
      ZachRigglemannequin
      on Jan 23, 2017
      MannequinAuthor

      Of note, there's no need to actually cause a SIGSEGV to generate the signal.

      The tests might be more clear to replace:

          libc.printf(ctypes.c_char_p(0xdeadbeef))

      with

      os.kill(os.getpid(), signal.SIGSEGV)

    14. vstinner commented on Jan 23, 2017

      @vstinner
      vstinner
      on Jan 23, 2017
      Member

      If you want crashes, look at the portable faulthandler._sigsegv() :-)

    15. ZachRiggle commented on Jan 23, 2017

      @ZachRiggle
      ZachRigglemannequin
      on Jan 23, 2017
      MannequinAuthor

      Neat, though that's not in the standard library.

      The current logic for getting a handle to libc could also be simplified via ctypes.util.find_library (https://docs.python.org/3/library/ctypes.html#finding-shared-libraries).

      Darwin:

          >>> import ctypes.util
    >>> ctypes.util.find_library('c')
    '/usr/lib/libc.dylib'
    
Linux:

    >>> import ctypes.util
    >>> ctypes.util.find_library('c')
    'libc.so.6'

    16. gpshead commented on May 23, 2017

      @gpshead
      gpshead
      on May 23, 2017
      Member

      New changeset 56bc3b7 by Gregory P. Smith in branch 'master':
      bpo-29335 - apply suggested test_subprocess simplifications from haypo and Zach: (bpo-1757)
      56bc3b7

    17. gpshead commented on May 24, 2017

      @gpshead
      gpshead
      on May 24, 2017
      Member

      Fixed applied to subprocess32 in google/python-subprocess32@0f1958e

    18. gpshead
      closed this as completedon May 24, 2017

    19. vstinner commented on Jun 26, 2017

      @vstinner
      vstinner
      on Jun 26, 2017
      Member

      New changeset cdee3f1 by Victor Stinner in branch 'master':
      bpo-30764: test_subprocess uses SuppressCrashReport (bpo-2405)
      cdee3f1

    20. vstinner commented on Jun 26, 2017

      @vstinner
      vstinner
      on Jun 26, 2017
      Member

      New changeset 849b062 by Victor Stinner in branch '3.5':
      bpo-30764: test_subprocess uses SuppressCrashReport (bpo-2405) (bpo-2411)
      849b062

    21. vstinner commented on Jun 26, 2017

      @vstinner
      vstinner
      on Jun 26, 2017
      Member

      New changeset 9ad50d9 by Victor Stinner in branch '3.6':
      bpo-30764: test_subprocess uses SuppressCrashReport (bpo-2405) (bpo-2410)
      9ad50d9

    22. vstinner commented on Jun 26, 2017

      @vstinner
      vstinner
      on Jun 26, 2017
      Member

      New changeset 2097b9e by Victor Stinner in branch '2.7':
      [2.7] bpo-30764: test_subprocess uses SuppressCrashReport (bpo-2405) (bpo-2412)
      2097b9e

    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

    Labels

    stdlibStandard Library Python modules in the Lib/ directorytype-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions