bpo-19764: Implemented support for subprocess.Popen(close_fds=True) on Windows (#1218)
Even though Python marks any handles it opens as non-inheritable there
is still a race when using `subprocess.Popen` since creating a process
with redirected stdio requires temporarily creating inheritable handles.
By implementing support for `subprocess.Popen(close_fds=True)` we fix
this race.
In order to implement this we use PROC_THREAD_ATTRIBUTE_HANDLE_LIST
which is available since Windows Vista. Which allows to pass an explicit
list of handles to inherit when creating a process.
This commit also adds `STARTUPINFO.lpAttributeList["handle_list"]`
which can be used to control PROC_THREAD_ATTRIBUTE_HANDLE_LIST
directly.
diff --git a/Modules/_winapi.c b/Modules/_winapi.c
index 604c05d..c596cba 100644
--- a/Modules/_winapi.c
+++ b/Modules/_winapi.c
@@ -811,6 +811,165 @@
return NULL;
}
+static LPHANDLE
+gethandlelist(PyObject *mapping, const char *name, Py_ssize_t *size)
+{
+ LPHANDLE ret = NULL;
+ PyObject *value_fast = NULL;
+ PyObject *value;
+ Py_ssize_t i;
+
+ value = PyMapping_GetItemString(mapping, name);
+ if (!value) {
+ PyErr_Clear();
+ return NULL;
+ }
+
+ if (value == Py_None) {
+ goto cleanup;
+ }
+
+ value_fast = PySequence_Fast(value, "handle_list must be a sequence or None");
+ if (value_fast == NULL)
+ goto cleanup;
+
+ *size = PySequence_Fast_GET_SIZE(value_fast) * sizeof(HANDLE);
+
+ /* Passing an empty array causes CreateProcess to fail so just don't set it */
+ if (*size == 0) {
+ goto cleanup;
+ }
+
+ 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));
+ if (ret[i] == (HANDLE)-1 && PyErr_Occurred()) {
+ PyMem_Free(ret);
+ ret = NULL;
+ goto cleanup;
+ }
+ }
+
+cleanup:
+ Py_DECREF(value);
+ Py_XDECREF(value_fast);
+ return ret;
+}
+
+typedef struct {
+ LPPROC_THREAD_ATTRIBUTE_LIST attribute_list;
+ LPHANDLE handle_list;
+} AttributeList;
+
+static void
+freeattributelist(AttributeList *attribute_list)
+{
+ if (attribute_list->attribute_list != NULL) {
+ DeleteProcThreadAttributeList(attribute_list->attribute_list);
+ PyMem_Free(attribute_list->attribute_list);
+ }
+
+ PyMem_Free(attribute_list->handle_list);
+
+ memset(attribute_list, 0, sizeof(*attribute_list));
+}
+
+static int
+getattributelist(PyObject *obj, const char *name, AttributeList *attribute_list)
+{
+ int ret = 0;
+ DWORD err;
+ BOOL result;
+ PyObject *value;
+ Py_ssize_t handle_list_size;
+ DWORD attribute_count = 0;
+ SIZE_T attribute_list_size = 0;
+
+ value = PyObject_GetAttrString(obj, name);
+ if (!value) {
+ PyErr_Clear(); /* FIXME: propagate error? */
+ return 0;
+ }
+
+ if (value == Py_None) {
+ ret = 0;
+ goto cleanup;
+ }
+
+ if (!PyMapping_Check(value)) {
+ ret = -1;
+ PyErr_Format(PyExc_TypeError, "%s must be a mapping or None", name);
+ goto cleanup;
+ }
+
+ attribute_list->handle_list = gethandlelist(value, "handle_list", &handle_list_size);
+ if (attribute_list->handle_list == NULL && PyErr_Occurred()) {
+ ret = -1;
+ goto cleanup;
+ }
+
+ if (attribute_list->handle_list != NULL)
+ ++attribute_count;
+
+ /* Get how many bytes we need for the attribute list */
+ result = InitializeProcThreadAttributeList(NULL, attribute_count, 0, &attribute_list_size);
+ if (result || GetLastError() != ERROR_INSUFFICIENT_BUFFER) {
+ ret = -1;
+ PyErr_SetFromWindowsErr(GetLastError());
+ goto cleanup;
+ }
+
+ attribute_list->attribute_list = PyMem_Malloc(attribute_list_size);
+ if (attribute_list->attribute_list == NULL) {
+ ret = -1;
+ goto cleanup;
+ }
+
+ result = InitializeProcThreadAttributeList(
+ attribute_list->attribute_list,
+ attribute_count,
+ 0,
+ &attribute_list_size);
+ if (!result) {
+ err = GetLastError();
+
+ /* So that we won't call DeleteProcThreadAttributeList */
+ PyMem_Free(attribute_list->attribute_list);
+ attribute_list->attribute_list = NULL;
+
+ ret = -1;
+ PyErr_SetFromWindowsErr(err);
+ goto cleanup;
+ }
+
+ if (attribute_list->handle_list != NULL) {
+ result = UpdateProcThreadAttribute(
+ attribute_list->attribute_list,
+ 0,
+ PROC_THREAD_ATTRIBUTE_HANDLE_LIST,
+ attribute_list->handle_list,
+ handle_list_size,
+ NULL,
+ NULL);
+ if (!result) {
+ ret = -1;
+ PyErr_SetFromWindowsErr(GetLastError());
+ goto cleanup;
+ }
+ }
+
+cleanup:
+ Py_DECREF(value);
+
+ if (ret < 0)
+ freeattributelist(attribute_list);
+
+ return ret;
+}
+
/*[clinic input]
_winapi.CreateProcess
@@ -842,34 +1001,35 @@
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;
+ AttributeList attribute_list = {0};
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 == NULL) {
- return NULL;
+ goto cleanup;
}
/* contains embedded null characters */
wenvironment = PyUnicode_AsUnicode(environment);
if (wenvironment == NULL) {
- Py_DECREF(environment);
- return NULL;
+ goto cleanup;
}
}
else {
@@ -877,29 +1037,41 @@
wenvironment = NULL;
}
+ if (getattributelist(startup_info, "lpAttributeList", &attribute_list) < 0)
+ goto cleanup;
+
+ si.lpAttributeList = attribute_list.attribute_list;
+
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
+ if (!result) {
+ PyErr_SetFromWindowsErr(GetLastError());
+ goto cleanup;
+ }
+
+ ret = Py_BuildValue("NNkk",
+ HANDLE_TO_PYNUM(pi.hProcess),
+ HANDLE_TO_PYNUM(pi.hThread),
+ pi.dwProcessId,
+ pi.dwThreadId);
+
+cleanup:
Py_XDECREF(environment);
+ freeattributelist(&attribute_list);
- if (! result)
- return PyErr_SetFromWindowsErr(GetLastError());
-
- return Py_BuildValue("NNkk",
- HANDLE_TO_PYNUM(pi.hProcess),
- HANDLE_TO_PYNUM(pi.hThread),
- pi.dwProcessId,
- pi.dwThreadId);
+ return ret;
}
/*[clinic input]
@@ -1489,7 +1661,6 @@
return Py_BuildValue("II", written, err);
}
-
/*[clinic input]
_winapi.GetACP
@@ -1503,6 +1674,30 @@
return PyLong_FromUnsignedLong(GetACP());
}
+/*[clinic input]
+_winapi.GetFileType -> DWORD
+
+ handle: HANDLE
+[clinic start generated code]*/
+
+static DWORD
+_winapi_GetFileType_impl(PyObject *module, HANDLE handle)
+/*[clinic end generated code: output=92b8466ac76ecc17 input=0058366bc40bbfbf]*/
+{
+ DWORD result;
+
+ Py_BEGIN_ALLOW_THREADS
+ result = GetFileType(handle);
+ Py_END_ALLOW_THREADS
+
+ if (result == FILE_TYPE_UNKNOWN && GetLastError() != NO_ERROR) {
+ PyErr_SetFromWindowsErr(0);
+ return -1;
+ }
+
+ return result;
+}
+
static PyMethodDef winapi_functions[] = {
_WINAPI_CLOSEHANDLE_METHODDEF
@@ -1530,6 +1725,7 @@
_WINAPI_WAITFORSINGLEOBJECT_METHODDEF
_WINAPI_WRITEFILE_METHODDEF
_WINAPI_GETACP_METHODDEF
+ _WINAPI_GETFILETYPE_METHODDEF
{NULL, NULL}
};
@@ -1623,6 +1819,12 @@
WINAPI_CONSTANT(F_DWORD, CREATE_DEFAULT_ERROR_MODE);
WINAPI_CONSTANT(F_DWORD, CREATE_BREAKAWAY_FROM_JOB);
+ WINAPI_CONSTANT(F_DWORD, FILE_TYPE_UNKNOWN);
+ WINAPI_CONSTANT(F_DWORD, FILE_TYPE_DISK);
+ WINAPI_CONSTANT(F_DWORD, FILE_TYPE_CHAR);
+ WINAPI_CONSTANT(F_DWORD, FILE_TYPE_PIPE);
+ WINAPI_CONSTANT(F_DWORD, FILE_TYPE_REMOTE);
+
WINAPI_CONSTANT("i", NULL);
return m;
diff --git a/Modules/clinic/_winapi.c.h b/Modules/clinic/_winapi.c.h
index 80edbf5..b14f087 100644
--- a/Modules/clinic/_winapi.c.h
+++ b/Modules/clinic/_winapi.c.h
@@ -907,4 +907,38 @@
{
return _winapi_GetACP_impl(module);
}
-/*[clinic end generated code: output=b9c00c323c9f4944 input=a9049054013a1b77]*/
+
+PyDoc_STRVAR(_winapi_GetFileType__doc__,
+"GetFileType($module, /, handle)\n"
+"--\n"
+"\n");
+
+#define _WINAPI_GETFILETYPE_METHODDEF \
+ {"GetFileType", (PyCFunction)_winapi_GetFileType, METH_FASTCALL|METH_KEYWORDS, _winapi_GetFileType__doc__},
+
+static DWORD
+_winapi_GetFileType_impl(PyObject *module, HANDLE handle);
+
+static PyObject *
+_winapi_GetFileType(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames)
+{
+ PyObject *return_value = NULL;
+ static const char * const _keywords[] = {"handle", NULL};
+ static _PyArg_Parser _parser = {"" F_HANDLE ":GetFileType", _keywords, 0};
+ HANDLE handle;
+ DWORD _return_value;
+
+ if (!_PyArg_ParseStackAndKeywords(args, nargs, kwnames, &_parser,
+ &handle)) {
+ goto exit;
+ }
+ _return_value = _winapi_GetFileType_impl(module, handle);
+ if ((_return_value == PY_DWORD_MAX) && PyErr_Occurred()) {
+ goto exit;
+ }
+ return_value = Py_BuildValue("k", _return_value);
+
+exit:
+ return return_value;
+}
+/*[clinic end generated code: output=ec7f36eb7efc9d00 input=a9049054013a1b77]*/