Skip to content

Commit ba1f9aa

Browse files
committed
gh-153962: Re-raise unexpected OSError in subprocess Popen._internal_poll
On POSIX, Popen._internal_poll() caught every OSError from os.waitpid() and, unless it was ECHILD or the call came from __del__ (a non-None _deadstate), silently discarded it, leaving returncode as None and masking a real failure. Add an else branch that re-raises such unexpected errors. The re-raise is only reachable when _deadstate is None; __del__ and _cleanup() always pass a non-None _deadstate, so __del__ still never raises. ECHILD handling is unchanged.
1 parent 32cfc88 commit ba1f9aa

3 files changed

Lines changed: 30 additions & 0 deletions

File tree

Lib/subprocess.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2191,6 +2191,13 @@ def _internal_poll(self, _deadstate=None, _del_safe=_del_safe):
21912191
# can't get the status.
21922192
# http://bugs.python.org/issue15756
21932193
self.returncode = 0
2194+
else:
2195+
# An unexpected error (e.g. EINVAL) must not be
2196+
# silently swallowed, leaving returncode as None:
2197+
# surface it to the caller. This branch is never
2198+
# reached from __del__, which always passes a
2199+
# non-None _deadstate.
2200+
raise
21942201
finally:
21952202
self._waitpid_lock.release()
21962203
return self.returncode

Lib/test/test_subprocess.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3419,6 +3419,25 @@ def test_leak_fast_process_del_killed(self):
34193419
else:
34203420
self.assertNotIn(ident, [id(o) for o in subprocess._active])
34213421

3422+
def test_internal_poll_reraises_unexpected_oserror(self):
3423+
# An unexpected OSError from waitpid() (i.e. not ECHILD, and not
3424+
# during the __del__/_deadstate path) must not be silently swallowed
3425+
# by poll(); doing so would leave returncode as None and mask a real
3426+
# failure.
3427+
p = subprocess.Popen([sys.executable, "-c", "import time; time.sleep(3)"])
3428+
self.addCleanup(p.wait)
3429+
self.addCleanup(p.kill)
3430+
3431+
def unexpected_waitpid(pid, flags):
3432+
raise OSError(errno.EINVAL, "simulated unexpected error")
3433+
3434+
with mock.patch.object(subprocess._del_safe, "waitpid",
3435+
unexpected_waitpid):
3436+
with self.assertRaises(OSError) as cm:
3437+
p.poll()
3438+
self.assertEqual(cm.exception.errno, errno.EINVAL)
3439+
self.assertIsNone(p.returncode)
3440+
34223441
def test_close_fds_after_preexec(self):
34233442
fd_status = support.findfile("fd_status.py", subdir="subprocessdata")
34243443

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
On POSIX, :meth:`subprocess.Popen.poll` now re-raises an unexpected
2+
:exc:`OSError` from the underlying :func:`os.waitpid` call instead of
3+
silently ignoring it and leaving :attr:`!returncode` set to ``None``. The
4+
special handling of :const:`~errno.ECHILD` is unchanged.

0 commit comments

Comments
 (0)