[Python-checkins] cpython (3.2): Fixes Issue #16114: The subprocess module no longer provides a
gregory.p.smith
python-checkins at python.org
Wed Oct 10 12:52:44 CEST 2012
http://hg.python.org/cpython/rev/e938fa6be509
changeset: 79632:e938fa6be509
branch: 3.2
parent: 79616:7de9c620716e
user: Gregory P. Smith <greg at krypto.org>
date: Wed Oct 10 03:34:47 2012 -0700
summary:
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.
files:
Lib/subprocess.py | 14 ++++++-
Lib/test/test_subprocess.py | 47 +++++++++++++++++++++---
Misc/NEWS | 4 ++
Modules/_posixsubprocess.c | 7 +++-
4 files changed, 63 insertions(+), 9 deletions(-)
diff --git a/Lib/subprocess.py b/Lib/subprocess.py
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -1169,6 +1169,7 @@
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 @@
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 @@
if start_new_session and hasattr(os, 'setsid'):
os.setsid()
+ reached_preexec = True
if preexec_fn:
preexec_fn()
@@ -1303,6 +1306,8 @@
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 @@
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)
diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -884,24 +884,30 @@
@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 @@
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.
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -129,6 +129,10 @@
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.
diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c
--- a/Modules/_posixsubprocess.c
+++ b/Modules/_posixsubprocess.c
@@ -354,7 +354,7 @@
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 @@
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 @@
}
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 {
--
Repository URL: http://hg.python.org/cpython
More information about the Python-checkins
mailing list