diff -r 459500606560 Doc/library/subprocess.rst --- a/Doc/library/subprocess.rst Sat Dec 31 11:40:11 2016 +0000 +++ b/Doc/library/subprocess.rst Fri Jan 06 14:34:59 2017 +0200 @@ -447,17 +447,20 @@ common use of *preexec_fn* to call os.setsid() in the child. If *close_fds* is true, all file descriptors except :const:`0`, :const:`1` and - :const:`2` will be closed before the child process is executed. (POSIX only). - The default varies by platform: Always true on POSIX. On Windows it is - true when *stdin*/*stdout*/*stderr* are :const:`None`, false otherwise. + :const:`2` will be closed before the child process is executed. On Windows, if *close_fds* is true then no handles will be inherited by the - child process. Note that on Windows, you cannot set *close_fds* to true and - also redirect the standard handles by setting *stdin*, *stdout* or *stderr*. + child process. .. versionchanged:: 3.2 The default for *close_fds* was changed from :const:`False` to what is described above. + .. versionchanged:: 3.7 + The default for *close_fds* was changed from :const:`False` to + :const:`True` on Windows. You can also set *close_fds* to :const:`True` + and redirect the standard handles by setting *stdin*, *stdout* or + *stderr*, which wasn't possible before 3.7 on Windows. + *pass_fds* is an optional sequence of file descriptors to keep open between the parent and child. Providing any *pass_fds* forces *close_fds* to be :const:`True`. (POSIX only) diff -r 459500606560 Lib/subprocess.py --- a/Lib/subprocess.py Sat Dec 31 11:40:11 2016 +0000 +++ b/Lib/subprocess.py Fri Jan 06 14:34:59 2017 +0200 @@ -132,6 +132,7 @@ hStdOutput = None hStdError = None wShowWindow = 0 + _handleList = None else: import _posixsubprocess import select @@ -610,17 +611,8 @@ if preexec_fn is not None: raise ValueError("preexec_fn is not supported on Windows " "platforms") - any_stdio_set = (stdin is not None or stdout is not None or - stderr is not None) if close_fds is _PLATFORM_DEFAULT_CLOSE_FDS: - if any_stdio_set: - close_fds = False - else: - close_fds = True - elif close_fds and any_stdio_set: - raise ValueError( - "close_fds is not supported on Windows platforms" - " if you redirect stdin/stdout/stderr") + close_fds = True else: # POSIX if close_fds is _PLATFORM_DEFAULT_CLOSE_FDS: @@ -963,6 +955,8 @@ if not isinstance(args, str): args = list2cmdline(args) + inherit_handles = int(not close_fds) + # Process startup details if startupinfo is None: startupinfo = STARTUPINFO() @@ -972,6 +966,11 @@ startupinfo.hStdOutput = c2pwrite startupinfo.hStdError = errwrite + # Only inherit the stdio handles + if close_fds: + inherit_handles = True + startupinfo._handleList = [p2cread, c2pwrite, errwrite] + if shell: startupinfo.dwFlags |= _winapi.STARTF_USESHOWWINDOW startupinfo.wShowWindow = _winapi.SW_HIDE @@ -983,7 +982,7 @@ hp, ht, pid, tid = _winapi.CreateProcess(executable, args, # no special security None, None, - int(not close_fds), + inherit_handles, creationflags, env, cwd, diff -r 459500606560 Lib/test/test_subprocess.py --- a/Lib/test/test_subprocess.py Sat Dec 31 11:40:11 2016 +0000 +++ b/Lib/test/test_subprocess.py Fri Jan 06 14:34:59 2017 +0200 @@ -2518,11 +2518,6 @@ [sys.executable, "-c", "import sys; sys.exit(47)"], preexec_fn=lambda: 1) - self.assertRaises(ValueError, subprocess.call, - [sys.executable, "-c", - "import sys; sys.exit(47)"], - stdout=subprocess.PIPE, - close_fds=True) def test_close_fds(self): # close file descriptors @@ -2531,6 +2526,32 @@ close_fds=True) self.assertEqual(rc, 47) + def test_close_fds_with_stdio(self): + import msvcrt + + fds = os.pipe() + self.addCleanup(os.close, fds[0]) + self.addCleanup(os.close, fds[1]) + + handles = [] + for fd in fds: + os.set_inheritable(fd, True) + handles.append(msvcrt.get_osfhandle(fd)) + + p = subprocess.Popen([sys.executable, "-c", + "import msvcrt; print(msvcrt.open_osfhandle({}, 0))".format(handles[0])], + stdout=subprocess.PIPE, close_fds=False) + stdout, stderr = p.communicate() + self.assertEqual(p.returncode, 0) + int(stdout.strip()) # Check that stdout is an integer + + p = subprocess.Popen([sys.executable, "-c", + "import msvcrt; print(msvcrt.open_osfhandle({}, 0))".format(handles[0])], + stdout=subprocess.PIPE, stderr=subprocess.PIPE, close_fds=True) + stdout, stderr = p.communicate() + self.assertEqual(p.returncode, 1) + self.assertIn(b"OSError", stderr) + def test_shell_sequence(self): # Run command through the shell (sequence) newenv = os.environ.copy() diff -r 459500606560 Modules/_winapi.c --- a/Modules/_winapi.c Sat Dec 31 11:40:11 2016 +0000 +++ b/Modules/_winapi.c Fri Jan 06 14:34:59 2017 +0200 @@ -790,6 +790,45 @@ return NULL; } +static LPHANDLE +gethandlelist(PyObject* obj, const char* name, Py_ssize_t* size) +{ + LPHANDLE ret = NULL; + PyObject* value_fast = NULL; + PyObject* value; + Py_ssize_t i; + + value = PyObject_GetAttrString(obj, name); + if (! value) { + PyErr_Clear(); /* FIXME: propagate error? */ + return NULL; + } + + if (value == Py_None) { + ret = NULL; + } + else { + value_fast = PySequence_Fast(value, "handleList must be a sequence or None"); + if (value_fast == NULL) + goto cleanup; + + *size = PySequence_Fast_GET_SIZE(value_fast) * sizeof(HANDLE); + + ret = PyMem_Malloc(*size); + if (ret == NULL) + goto cleanup; + + for (i = 0; i < PySequence_Fast_GET_SIZE(value_fast); i++) { + ret[i] = PYNUM_TO_HANDLE(PySequence_Fast_GET_ITEM(value_fast, i)); + } + } + +cleanup: + Py_DECREF(value); + Py_XDECREF(value_fast); + return ret; +} + /*[clinic input] _winapi.CreateProcess @@ -821,63 +860,114 @@ PyObject *startup_info) /*[clinic end generated code: output=4652a33aff4b0ae1 input=4a43b05038d639bb]*/ { + PyObject* ret = NULL; BOOL result; PROCESS_INFORMATION pi; - STARTUPINFOW si; - PyObject* environment; + STARTUPINFOEXW si; + PyObject* environment = NULL; wchar_t *wenvironment; + Py_ssize_t handle_list_size; + LPHANDLE handle_list = NULL; + SIZE_T attribute_list_size; ZeroMemory(&si, sizeof(si)); - si.cb = sizeof(si); + si.StartupInfo.cb = sizeof(si); /* note: we only support a small subset of all SI attributes */ - si.dwFlags = getulong(startup_info, "dwFlags"); - si.wShowWindow = (WORD)getulong(startup_info, "wShowWindow"); - si.hStdInput = gethandle(startup_info, "hStdInput"); - si.hStdOutput = gethandle(startup_info, "hStdOutput"); - si.hStdError = gethandle(startup_info, "hStdError"); + si.StartupInfo.dwFlags = getulong(startup_info, "dwFlags"); + si.StartupInfo.wShowWindow = (WORD)getulong(startup_info, "wShowWindow"); + si.StartupInfo.hStdInput = gethandle(startup_info, "hStdInput"); + si.StartupInfo.hStdOutput = gethandle(startup_info, "hStdOutput"); + si.StartupInfo.hStdError = gethandle(startup_info, "hStdError"); if (PyErr_Occurred()) - return NULL; + goto cleanup; if (env_mapping != Py_None) { environment = getenvironment(env_mapping); if (! environment) - return NULL; + goto cleanup; wenvironment = PyUnicode_AsUnicode(environment); if (wenvironment == NULL) - { - Py_XDECREF(environment); - return NULL; - } + goto cleanup; } else { environment = NULL; wenvironment = NULL; } + handle_list = gethandlelist(startup_info, "_handleList", &handle_list_size); + if (PyErr_Occurred()) + goto cleanup; + + /* build si.lpAttributeList if handle_list is not NULL */ + if (handle_list != NULL) { + result = InitializeProcThreadAttributeList(NULL, 1, 0, &attribute_list_size); + if (result || GetLastError() != ERROR_INSUFFICIENT_BUFFER) { + PyErr_SetFromWindowsErr(GetLastError()); + goto cleanup; + } + + si.lpAttributeList = PyMem_Malloc(attribute_list_size); + if (si.lpAttributeList == NULL) + goto cleanup; + + result = InitializeProcThreadAttributeList(si.lpAttributeList, 1, 0, &attribute_list_size); + if (! result) { + /* So that we won't call DeleteProcThreadAttributeList */ + PyMem_Free(si.lpAttributeList); + si.lpAttributeList = NULL; + PyErr_SetFromWindowsErr(GetLastError()); + goto cleanup; + } + + result = UpdateProcThreadAttribute( + si.lpAttributeList, + 0, + PROC_THREAD_ATTRIBUTE_HANDLE_LIST, + handle_list, + handle_list_size, + NULL, + NULL); + if (! result) { + PyErr_SetFromWindowsErr(GetLastError()); + goto cleanup; + } + } + Py_BEGIN_ALLOW_THREADS result = CreateProcessW(application_name, command_line, NULL, NULL, inherit_handles, - creation_flags | CREATE_UNICODE_ENVIRONMENT, + creation_flags | EXTENDED_STARTUPINFO_PRESENT | CREATE_UNICODE_ENVIRONMENT, wenvironment, current_directory, - &si, + (LPSTARTUPINFOW)&si, &pi); Py_END_ALLOW_THREADS - Py_XDECREF(environment); - - if (! result) - return PyErr_SetFromWindowsErr(GetLastError()); + if (! result) { + PyErr_SetFromWindowsErr(GetLastError()); + goto cleanup; + } - return Py_BuildValue("NNkk", - HANDLE_TO_PYNUM(pi.hProcess), - HANDLE_TO_PYNUM(pi.hThread), - pi.dwProcessId, - pi.dwThreadId); + ret = Py_BuildValue("NNkk", + HANDLE_TO_PYNUM(pi.hProcess), + HANDLE_TO_PYNUM(pi.hThread), + pi.dwProcessId, + pi.dwThreadId); + +cleanup: + Py_XDECREF(environment); + PyMem_Free(handle_list); + + if (si.lpAttributeList != NULL) { + DeleteProcThreadAttributeList(si.lpAttributeList); + PyMem_Free(si.lpAttributeList); + } + + return ret; } /*[clinic input]