On POSIX, subprocess.Popen.poll() can quietly hide a real error.
Internally poll() calls os.waitpid() and catches OSError. It handles two cases: ECHILD (the child was already reaped) and the case where the call comes from __del__. If waitpid() fails with anything else, such as EINVAL, the exception is caught and dropped. returncode stays None, so poll() returns None as if the process were still running and the real failure disappears.
Here is a way to reproduce it. Note that poll() uses a copy of os.waitpid that subprocess saves at import time, so you have to patch it through subprocess._del_safe, not os.waitpid:
import subprocess, errno
from unittest import mock
p = subprocess.Popen(["sleep", "30"])
def fake_waitpid(pid, flags):
raise OSError(errno.EINVAL, "simulated")
with mock.patch.object(subprocess._del_safe, "waitpid", fake_waitpid):
print(p.poll(), p.returncode) # prints: None None
p.kill(); p.wait()The fix is to re-raise the unexpected errors instead of ignoring them. The __del__ path is handled first (it passes a deadstate value), so it stays unaffected, and the ECHILD handling doesn't change.
Found while going through devdanzin's audit of the standard library, item 8: https://gist.github.com/devdanzin/3198710e3c0128fda5e0a7b4e0768e5f
On POSIX,
subprocess.Popen.poll()can quietly hide a real error.Internally
poll()callsos.waitpid()and catchesOSError. It handles two cases:ECHILD(the child was already reaped) and the case where the call comes from__del__. Ifwaitpid()fails with anything else, such asEINVAL, the exception is caught and dropped.returncodestaysNone, sopoll()returnsNoneas if the process were still running and the real failure disappears.Here is a way to reproduce it. Note that
poll()uses a copy ofos.waitpidthat subprocess saves at import time, so you have to patch it throughsubprocess._del_safe, notos.waitpid:The fix is to re-raise the unexpected errors instead of ignoring them. The
__del__path is handled first (it passes a deadstate value), so it stays unaffected, and theECHILDhandling doesn't change.Found while going through devdanzin's audit of the standard library, item 8: https://gist.github.com/devdanzin/3198710e3c0128fda5e0a7b4e0768e5f