From 8d07c264e4bcaaf1fe3b9fc92ea5730efe13eaa2 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Sat, 10 Nov 2012 23:53:47 -0800 Subject: [PATCH] Raise our own SubprocessError rather than a RuntimeError in when dealing with odd rare errors coming from the subprocess module. --- Lib/subprocess.py | 12 ++++++------ Lib/test/test_subprocess.py | 6 +++--- Misc/NEWS | 3 +++ Modules/_posixsubprocess.c | 2 +- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index db0ba190a63..d52f9a45f39 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -1042,9 +1042,9 @@ class Popen(object): w9xpopen = os.path.join(os.path.dirname(sys.base_exec_prefix), "w9xpopen.exe") if not os.path.exists(w9xpopen): - raise RuntimeError("Cannot locate w9xpopen.exe, which is " - "needed for Popen to work with your " - "shell or platform.") + raise SubprocessError( + "Cannot locate w9xpopen.exe, which is needed for " + "Popen to work with your shell or platform.") return w9xpopen @@ -1414,12 +1414,12 @@ class Popen(object): except ValueError: warnings.warn(RuntimeWarning( 'Bad exception data: %r' % errpipe_data)) - exception_name = b'RuntimeError' + exception_name = b'SubprocessError' hex_errno = b'0' err_msg = b'Unknown' child_exception_type = getattr( builtins, exception_name.decode('ascii'), - RuntimeError) + SubprocessError) for fd in (p2cwrite, c2pread, errread): if fd != -1: os.close(fd) @@ -1452,7 +1452,7 @@ class Popen(object): self.returncode = _WEXITSTATUS(sts) else: # Should never happen - raise RuntimeError("Unknown child exit status!") + raise SubprocessError("Unknown child exit status!") def _internal_poll(self, _deadstate=None, _waitpid=os.waitpid, diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 2d77554906f..8978b31fb5f 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1179,7 +1179,7 @@ class POSIXProcessTestCase(BaseTestCase): try: p = subprocess.Popen([sys.executable, "-c", ""], preexec_fn=raise_it) - except RuntimeError as e: + except subprocess.SubprocessError as e: self.assertTrue( subprocess._posixsubprocess, "Expected a ValueError from the preexec_fn") @@ -1544,12 +1544,12 @@ class POSIXProcessTestCase(BaseTestCase): # Pure Python implementations keeps the message self.assertIsNone(subprocess._posixsubprocess) self.assertEqual(str(err), "surrogate:\uDCff") - except RuntimeError as err: + except subprocess.SubprocessError as err: # _posixsubprocess uses a default message self.assertIsNotNone(subprocess._posixsubprocess) self.assertEqual(str(err), "Exception occurred in preexec_fn.") else: - self.fail("Expected ValueError or RuntimeError") + self.fail("Expected ValueError or subprocess.SubprocessError") def test_undecodable_env(self): for key, value in (('test', 'abc\uDCFF'), ('test\uDCFF', '42')): diff --git a/Misc/NEWS b/Misc/NEWS index 2187c80d500..30e32173683 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -116,6 +116,9 @@ Core and Builtins Library ------- +- The subprocess module now raises its own SubprocessError instead of a + RuntimeError in various error situations which should not normally happen. + - Issue #16327: The subprocess module no longer leaks file descriptors used for stdin/stdout/stderr pipes to the child when fork() fails. diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index b7b120ba510..4ccd38bb6d6 100644 --- a/Modules/_posixsubprocess.c +++ b/Modules/_posixsubprocess.c @@ -497,7 +497,7 @@ error: /* We can't call strerror(saved_errno). It is not async signal safe. * The parent process will look the error message up. */ } else { - unused = write(errpipe_write, "RuntimeError:0:", 15); + unused = write(errpipe_write, "SubprocessError:0:", 18); unused = write(errpipe_write, err_msg, strlen(err_msg)); } if (unused) return; /* silly? yes! avoids gcc compiler warning. */