Fixes Issue #16114: The subprocess module no longer provides a

misleading error message stating that args[0] did not exist when
either the cwd or executable keyword arguments specified a path that
did not exist.

It now keeps track of if the child got as far as preexec and reports it if
not back to the parent via a special "noexec" error message value in
the error pipe so that the cwd can be blamed for a failed chdir
instead of the exec of the executable being blamed instead.

The executable is also always reported accurately when exec fails.

Unittests enhanced to cover these cases.
This commit is contained in:
Gregory P. Smith 2012-10-10 03:34:47 -07:00
parent a256841b4b
commit 5591b02a4c
4 changed files with 63 additions and 9 deletions

View file

@ -1169,6 +1169,7 @@ def _execute_child(self, args, executable, preexec_fn, close_fds,
if executable is None:
executable = args[0]
orig_executable = executable
# For transferring possible exec failure from child to parent.
# Data format: "exception name:hex errno:description"
@ -1224,6 +1225,7 @@ def _execute_child(self, args, executable, preexec_fn, close_fds,
self._child_created = True
if self.pid == 0:
# Child
reached_preexec = False
try:
# Close parent's pipe ends
if p2cwrite != -1:
@ -1288,6 +1290,7 @@ def _dup2(a, b):
if start_new_session and hasattr(os, 'setsid'):
os.setsid()
reached_preexec = True
if preexec_fn:
preexec_fn()
@ -1303,6 +1306,8 @@ def _dup2(a, b):
errno_num = exc_value.errno
else:
errno_num = 0
if not reached_preexec:
exc_value = "noexec"
message = '%s:%x:%s' % (exc_type.__name__,
errno_num, exc_value)
message = message.encode(errors="surrogatepass")
@ -1364,10 +1369,17 @@ def _dup2(a, b):
err_msg = err_msg.decode(errors="surrogatepass")
if issubclass(child_exception_type, OSError) and hex_errno:
errno_num = int(hex_errno, 16)
child_exec_never_called = (err_msg == "noexec")
if child_exec_never_called:
err_msg = ""
if errno_num != 0:
err_msg = os.strerror(errno_num)
if errno_num == errno.ENOENT:
err_msg += ': ' + repr(args[0])
if child_exec_never_called:
# The error must be from chdir(cwd).
err_msg += ': ' + repr(cwd)
else:
err_msg += ': ' + repr(orig_executable)
raise child_exception_type(errno_num, err_msg)
raise child_exception_type(err_msg)

View file

@ -884,24 +884,30 @@ def __exit__(self, *args):
@unittest.skipIf(mswindows, "POSIX specific tests")
class POSIXProcessTestCase(BaseTestCase):
def test_exceptions(self):
nonexistent_dir = "/_this/pa.th/does/not/exist"
def setUp(self):
super().setUp()
self._nonexistent_dir = "/_this/pa.th/does/not/exist"
def _get_chdir_exception(self):
try:
os.chdir(nonexistent_dir)
os.chdir(self._nonexistent_dir)
except OSError as e:
# This avoids hard coding the errno value or the OS perror()
# string and instead capture the exception that we want to see
# below for comparison.
desired_exception = e
desired_exception.strerror += ': ' + repr(sys.executable)
desired_exception.strerror += ': ' + repr(self._nonexistent_dir)
else:
self.fail("chdir to nonexistant directory %s succeeded." %
nonexistent_dir)
self._nonexistent_dir)
return desired_exception
# Error in the child re-raised in the parent.
def test_exception_cwd(self):
"""Test error in the child raised in the parent for a bad cwd."""
desired_exception = self._get_chdir_exception()
try:
p = subprocess.Popen([sys.executable, "-c", ""],
cwd=nonexistent_dir)
cwd=self._nonexistent_dir)
except OSError as e:
# Test that the child process chdir failure actually makes
# it up to the parent process as the correct exception.
@ -910,6 +916,33 @@ def test_exceptions(self):
else:
self.fail("Expected OSError: %s" % desired_exception)
def test_exception_bad_executable(self):
"""Test error in the child raised in the parent for a bad executable."""
desired_exception = self._get_chdir_exception()
try:
p = subprocess.Popen([sys.executable, "-c", ""],
executable=self._nonexistent_dir)
except OSError as e:
# Test that the child process exec failure actually makes
# it up to the parent process as the correct exception.
self.assertEqual(desired_exception.errno, e.errno)
self.assertEqual(desired_exception.strerror, e.strerror)
else:
self.fail("Expected OSError: %s" % desired_exception)
def test_exception_bad_args_0(self):
"""Test error in the child raised in the parent for a bad args[0]."""
desired_exception = self._get_chdir_exception()
try:
p = subprocess.Popen([self._nonexistent_dir, "-c", ""])
except OSError as e:
# Test that the child process exec failure actually makes
# it up to the parent process as the correct exception.
self.assertEqual(desired_exception.errno, e.errno)
self.assertEqual(desired_exception.strerror, e.strerror)
else:
self.fail("Expected OSError: %s" % desired_exception)
def test_restore_signals(self):
# Code coverage for both values of restore_signals to make sure it
# at least does not blow up.

View file

@ -129,6 +129,10 @@ Core and Builtins
Library
-------
- Issue #16114: The subprocess module no longer provides a misleading
error message stating that args[0] did not exist when either the cwd or
executable keyword arguments specified a path that did not exist.
- Issue #15756: subprocess.poll() now properly handles errno.ECHILD to
return a returncode of 0 when the child has already exited or cannot
be waited on.

View file

@ -354,7 +354,7 @@ child_exec(char *const exec_array[],
PyObject *preexec_fn,
PyObject *preexec_fn_args_tuple)
{
int i, saved_errno, unused;
int i, saved_errno, unused, reached_preexec = 0;
PyObject *result;
const char* err_msg = "";
/* Buffer large enough to hold a hex integer. We can't malloc. */
@ -438,6 +438,7 @@ child_exec(char *const exec_array[],
POSIX_CALL(setsid());
#endif
reached_preexec = 1;
if (preexec_fn != Py_None && preexec_fn_args_tuple) {
/* This is where the user has asked us to deadlock their program. */
result = PyObject_Call(preexec_fn, preexec_fn_args_tuple, NULL);
@ -487,6 +488,10 @@ child_exec(char *const exec_array[],
}
unused = write(errpipe_write, cur, hex_errno + sizeof(hex_errno) - cur);
unused = write(errpipe_write, ":", 1);
if (!reached_preexec) {
/* Indicate to the parent that the error happened before exec(). */
unused = write(errpipe_write, "noexec", 6);
}
/* We can't call strerror(saved_errno). It is not async signal safe.
* The parent process will look the error message up. */
} else {