diff --git a/Lib/subprocess.py b/Lib/subprocess.py index c461b254f6d..638058ee61f 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -638,21 +638,13 @@ class Popen(object): c2pread, c2pwrite, errread, errwrite) - # On Windows, you cannot just redirect one or two handles: You - # either have to redirect all three or none. If the subprocess - # user has only redirected one or two handles, we are - # automatically creating PIPEs for the rest. We should close - # these after the process is started. See bug #1124861. if mswindows: - if stdin is None and p2cwrite is not None: - os.close(p2cwrite) - p2cwrite = None - if stdout is None and c2pread is not None: - os.close(c2pread) - c2pread = None - if stderr is None and errread is not None: - os.close(errread) - errread = None + if p2cwrite is not None: + p2cwrite = msvcrt.open_osfhandle(p2cwrite.Detach(), 0) + if c2pread is not None: + c2pread = msvcrt.open_osfhandle(c2pread.Detach(), 0) + if errread is not None: + errread = msvcrt.open_osfhandle(errread.Detach(), 0) if bufsize == 0: bufsize = 1 # Nearly unbuffered (XXX for now) @@ -737,13 +729,10 @@ class Popen(object): if stdin is None: p2cread = GetStdHandle(STD_INPUT_HANDLE) - if p2cread is not None: - pass - elif stdin is None or stdin == PIPE: + if p2cread is None: + p2cread, _ = CreatePipe(None, 0) + elif stdin == PIPE: p2cread, p2cwrite = CreatePipe(None, 0) - # Detach and turn into fd - p2cwrite = p2cwrite.Detach() - p2cwrite = msvcrt.open_osfhandle(p2cwrite, 0) elif isinstance(stdin, int): p2cread = msvcrt.get_osfhandle(stdin) else: @@ -753,13 +742,10 @@ class Popen(object): if stdout is None: c2pwrite = GetStdHandle(STD_OUTPUT_HANDLE) - if c2pwrite is not None: - pass - elif stdout is None or stdout == PIPE: + if c2pwrite is None: + _, c2pwrite = CreatePipe(None, 0) + elif stdout == PIPE: c2pread, c2pwrite = CreatePipe(None, 0) - # Detach and turn into fd - c2pread = c2pread.Detach() - c2pread = msvcrt.open_osfhandle(c2pread, 0) elif isinstance(stdout, int): c2pwrite = msvcrt.get_osfhandle(stdout) else: @@ -769,13 +755,10 @@ class Popen(object): if stderr is None: errwrite = GetStdHandle(STD_ERROR_HANDLE) - if errwrite is not None: - pass - elif stderr is None or stderr == PIPE: + if errwrite is None: + _, errwrite = CreatePipe(None, 0) + elif stderr == PIPE: errread, errwrite = CreatePipe(None, 0) - # Detach and turn into fd - errread = errread.Detach() - errread = msvcrt.open_osfhandle(errread, 0) elif stderr == STDOUT: errwrite = c2pwrite elif isinstance(stderr, int): diff --git a/Misc/NEWS b/Misc/NEWS index 0a7a53b72c6..702e02f119c 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -181,6 +181,8 @@ Core and Builtins Library ------- +- Issue #5179: Fixed subprocess handle leak on failure on windows. + - PEP 372: Added collections.OrderedDict(). - The _asdict() for method for namedtuples now returns an OrderedDict(). diff --git a/PC/_subprocess.c b/PC/_subprocess.c index 99cbd24459d..d0c984398e5 100644 --- a/PC/_subprocess.c +++ b/PC/_subprocess.c @@ -84,7 +84,7 @@ sp_handle_detach(sp_handle_object* self, PyObject* args) handle = self->handle; - self->handle = NULL; + self->handle = INVALID_HANDLE_VALUE; /* note: return the current handle, as an integer */ return HANDLE_TO_PYNUM(handle);