[Python-checkins] bpo-19764: Implemented support for subprocess.Popen(close_fds=True) on Windows (#1218)

Victor Stinner webhook-mailer at python.org
Mon Dec 18 04:28:28 EST 2017


https://github.com/python/cpython/commit/b2a6083eb0384f38839d3f1ed32262a3852026fa
commit: b2a6083eb0384f38839d3f1ed32262a3852026fa
branch: master
author: Segev Finer <segev208 at gmail.com>
committer: Victor Stinner <victor.stinner at gmail.com>
date: 2017-12-18T10:28:19+01:00
summary:

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.

files:
A Misc/NEWS.d/next/Windows/2017-08-18-18-00-24.bpo-19764.ODpc9y.rst
M Doc/library/subprocess.rst
M Doc/whatsnew/3.7.rst
M Lib/subprocess.py
M Lib/test/test_subprocess.py
M Misc/ACKS
M Modules/_winapi.c
M Modules/clinic/_winapi.c.h

diff --git a/Doc/library/subprocess.rst b/Doc/library/subprocess.rst
index a2c184a0460..af96f41ef40 100644
--- a/Doc/library/subprocess.rst
+++ b/Doc/library/subprocess.rst
@@ -452,17 +452,20 @@ functions.
       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 unless explicitly passed in the ``handle_list`` element of
+   :attr:`STARTUPINFO.lpAttributeList`, or by standard handle redirection.
 
    .. versionchanged:: 3.2
       The default for *close_fds* was changed from :const:`False` to
       what is described above.
 
+   .. versionchanged:: 3.7
+      On Windows the default for *close_fds* was changed from :const:`False` to
+      :const:`True` when redirecting the standard handles. It's now possible to
+      set *close_fds* to :const:`True` when redirecting the standard handles.
+
    *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)
@@ -764,7 +767,7 @@ The :class:`STARTUPINFO` class and following constants are only available
 on Windows.
 
 .. class:: STARTUPINFO(*, dwFlags=0, hStdInput=None, hStdOutput=None, \
-                       hStdError=None, wShowWindow=0)
+                       hStdError=None, wShowWindow=0, lpAttributeList=None)
 
    Partial support of the Windows
    `STARTUPINFO <https://msdn.microsoft.com/en-us/library/ms686331(v=vs.85).aspx>`__
@@ -814,6 +817,33 @@ on Windows.
       :data:`SW_HIDE` is provided for this attribute. It is used when
       :class:`Popen` is called with ``shell=True``.
 
+   .. attribute:: lpAttributeList
+
+      A dictionary of additional attributes for process creation as given in
+      ``STARTUPINFOEX``, see
+      `UpdateProcThreadAttribute <https://msdn.microsoft.com/en-us/library/windows/desktop/ms686880(v=vs.85).aspx>`__.
+
+      Supported attributes:
+
+      **handle_list**
+         Sequence of handles that will be inherited. *close_fds* must be true if
+         non-empty.
+
+         The handles must be temporarily made inheritable by
+         :func:`os.set_handle_inheritable` when passed to the :class:`Popen`
+         constructor, else :class:`OSError` will be raised with Windows error
+         ``ERROR_INVALID_PARAMETER`` (87).
+
+         .. warning::
+
+            In a multithreaded process, use caution to avoid leaking handles
+            that are marked inheritable when combining this feature with
+            concurrent calls to other process creation functions that inherit
+            all handles such as :func:`os.system`.  This also applies to
+            standard handle redirection, which temporarily creates inheritable
+            handles.
+
+      .. versionadded:: 3.7
 
 Windows Constants
 ^^^^^^^^^^^^^^^^^
diff --git a/Doc/whatsnew/3.7.rst b/Doc/whatsnew/3.7.rst
index 3574b53ad18..82f7cc07043 100644
--- a/Doc/whatsnew/3.7.rst
+++ b/Doc/whatsnew/3.7.rst
@@ -437,6 +437,17 @@ string
 expression pattern for braced placeholders and non-braced placeholders
 separately.  (Contributed by Barry Warsaw in :issue:`1198569`.)
 
+subprocess
+----------
+
+On Windows the default for *close_fds* was changed from :const:`False` to
+:const:`True` when redirecting the standard handles. It's now possible to set
+*close_fds* to :const:`True` when redirecting the standard handles. See
+:class:`subprocess.Popen`.
+
+This means that *close_fds* now defaults to :const:`True` on all supported
+platforms.
+
 sys
 ---
 
@@ -883,6 +894,14 @@ Changes in the Python API
 
 .. _Unicode Technical Standard #18: https://unicode.org/reports/tr18/
 
+* On Windows the default for the *close_fds* argument of
+  :class:`subprocess.Popen` was changed from :const:`False` to :const:`True`
+  when redirecting the standard handles. If you previously depended on handles
+  being inherited when using :class:`subprocess.Popen` with standard io
+  redirection, you will have to pass ``close_fds=False`` to preserve the
+  previous behaviour, or use
+  :attr:`STARTUPINFO.lpAttributeList <subprocess.STARTUPINFO.lpAttributeList>`.
+
 
 Changes in the C API
 --------------------
diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index 65b4086dc61..db6342fa491 100644
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -128,12 +128,13 @@ def stdout(self, value):
     import _winapi
     class STARTUPINFO:
         def __init__(self, *, dwFlags=0, hStdInput=None, hStdOutput=None,
-                     hStdError=None, wShowWindow=0):
+                     hStdError=None, wShowWindow=0, lpAttributeList=None):
             self.dwFlags = dwFlags
             self.hStdInput = hStdInput
             self.hStdOutput = hStdOutput
             self.hStdError = hStdError
             self.wShowWindow = wShowWindow
+            self.lpAttributeList = lpAttributeList or {"handle_list": []}
 else:
     import _posixsubprocess
     import select
@@ -577,9 +578,6 @@ def getoutput(cmd):
     return getstatusoutput(cmd)[1]
 
 
-_PLATFORM_DEFAULT_CLOSE_FDS = object()
-
-
 class Popen(object):
     """ Execute a child program in a new process.
 
@@ -630,7 +628,7 @@ class Popen(object):
 
     def __init__(self, args, bufsize=-1, executable=None,
                  stdin=None, stdout=None, stderr=None,
-                 preexec_fn=None, close_fds=_PLATFORM_DEFAULT_CLOSE_FDS,
+                 preexec_fn=None, close_fds=True,
                  shell=False, cwd=None, env=None, universal_newlines=None,
                  startupinfo=None, creationflags=0,
                  restore_signals=True, start_new_session=False,
@@ -655,21 +653,8 @@ def __init__(self, args, bufsize=-1, executable=None,
             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")
         else:
             # POSIX
-            if close_fds is _PLATFORM_DEFAULT_CLOSE_FDS:
-                close_fds = True
             if pass_fds and not close_fds:
                 warnings.warn("pass_fds overriding close_fds.", RuntimeWarning)
                 close_fds = True
@@ -1019,6 +1004,19 @@ def _make_inheritable(self, handle):
             return Handle(h)
 
 
+        def _filter_handle_list(self, handle_list):
+            """Filter out console handles that can't be used
+            in lpAttributeList["handle_list"] and make sure the list
+            isn't empty. This also removes duplicate handles."""
+            # An handle with it's lowest two bits set might be a special console
+            # handle that if passed in lpAttributeList["handle_list"], will
+            # cause it to fail.
+            return list({handle for handle in handle_list
+                         if handle & 0x3 != 0x3
+                         or _winapi.GetFileType(handle) !=
+                            _winapi.FILE_TYPE_CHAR})
+
+
         def _execute_child(self, args, executable, preexec_fn, close_fds,
                            pass_fds, cwd, env,
                            startupinfo, creationflags, shell,
@@ -1036,12 +1034,41 @@ def _execute_child(self, args, executable, preexec_fn, close_fds,
             # Process startup details
             if startupinfo is None:
                 startupinfo = STARTUPINFO()
-            if -1 not in (p2cread, c2pwrite, errwrite):
+
+            use_std_handles = -1 not in (p2cread, c2pwrite, errwrite)
+            if use_std_handles:
                 startupinfo.dwFlags |= _winapi.STARTF_USESTDHANDLES
                 startupinfo.hStdInput = p2cread
                 startupinfo.hStdOutput = c2pwrite
                 startupinfo.hStdError = errwrite
 
+            attribute_list = startupinfo.lpAttributeList
+            have_handle_list = bool(attribute_list and
+                                    "handle_list" in attribute_list and
+                                    attribute_list["handle_list"])
+
+            # If we were given an handle_list or need to create one
+            if have_handle_list or (use_std_handles and close_fds):
+                if attribute_list is None:
+                    attribute_list = startupinfo.lpAttributeList = {}
+                handle_list = attribute_list["handle_list"] = \
+                    list(attribute_list.get("handle_list", []))
+
+                if use_std_handles:
+                    handle_list += [int(p2cread), int(c2pwrite), int(errwrite)]
+
+                handle_list[:] = self._filter_handle_list(handle_list)
+
+                if handle_list:
+                    if not close_fds:
+                        warnings.warn("startupinfo.lpAttributeList['handle_list'] "
+                                      "overriding close_fds", RuntimeWarning)
+
+                    # When using the handle_list we always request to inherit
+                    # handles but the only handles that will be inherited are
+                    # the ones in the handle_list
+                    close_fds = False
+
             if shell:
                 startupinfo.dwFlags |= _winapi.STARTF_USESHOWWINDOW
                 startupinfo.wShowWindow = _winapi.SW_HIDE
diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
index fff1b0db105..bd3b9b46f76 100644
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -2743,11 +2743,6 @@ def test_invalid_args(self):
                           [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)
 
     @support.cpython_only
     def test_issue31471(self):
@@ -2765,6 +2760,67 @@ def test_close_fds(self):
                               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)
+
+        # The same as the previous call, but with an empty handle_list
+        handle_list = []
+        startupinfo = subprocess.STARTUPINFO()
+        startupinfo.lpAttributeList = {"handle_list": handle_list}
+        p = subprocess.Popen([sys.executable, "-c",
+                              "import msvcrt; print(msvcrt.open_osfhandle({}, 0))".format(handles[0])],
+                             stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+                             startupinfo=startupinfo, close_fds=True)
+        stdout, stderr = p.communicate()
+        self.assertEqual(p.returncode, 1)
+        self.assertIn(b"OSError", stderr)
+
+        # Check for a warning due to using handle_list and close_fds=False
+        with support.check_warnings((".*overriding close_fds", RuntimeWarning)):
+            startupinfo = subprocess.STARTUPINFO()
+            startupinfo.lpAttributeList = {"handle_list": handles[:]}
+            p = subprocess.Popen([sys.executable, "-c",
+                                  "import msvcrt; print(msvcrt.open_osfhandle({}, 0))".format(handles[0])],
+                                 stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+                                 startupinfo=startupinfo, close_fds=False)
+            stdout, stderr = p.communicate()
+            self.assertEqual(p.returncode, 0)
+
+    def test_empty_attribute_list(self):
+        startupinfo = subprocess.STARTUPINFO()
+        startupinfo.lpAttributeList = {}
+        subprocess.call([sys.executable, "-c", "import sys; sys.exit(0)"],
+                        startupinfo=startupinfo)
+
+    def test_empty_handle_list(self):
+        startupinfo = subprocess.STARTUPINFO()
+        startupinfo.lpAttributeList = {"handle_list": []}
+        subprocess.call([sys.executable, "-c", "import sys; sys.exit(0)"],
+                        startupinfo=startupinfo)
+
     def test_shell_sequence(self):
         # Run command through the shell (sequence)
         newenv = os.environ.copy()
diff --git a/Misc/ACKS b/Misc/ACKS
index 8303ce80050..e5343899640 100644
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -466,6 +466,7 @@ Carl Feynman
 Vincent Fiack
 Anastasia Filatova
 Tomer Filiba
+Segev Finer
 Jeffrey Finkelstein
 Russell Finn
 Dan Finnie
diff --git a/Misc/NEWS.d/next/Windows/2017-08-18-18-00-24.bpo-19764.ODpc9y.rst b/Misc/NEWS.d/next/Windows/2017-08-18-18-00-24.bpo-19764.ODpc9y.rst
new file mode 100644
index 00000000000..b5f5cae8323
--- /dev/null
+++ b/Misc/NEWS.d/next/Windows/2017-08-18-18-00-24.bpo-19764.ODpc9y.rst
@@ -0,0 +1,2 @@
+Implement support for `subprocess.Popen(close_fds=True)` on Windows. Patch
+by Segev Finer.
diff --git a/Modules/_winapi.c b/Modules/_winapi.c
index 604c05d4ae1..c596cba3cbc 100644
--- a/Modules/_winapi.c
+++ b/Modules/_winapi.c
@@ -811,6 +811,165 @@ getenvironment(PyObject* environment)
     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 @@ _winapi_CreateProcess_impl(PyObject *module, Py_UNICODE *application_name,
                            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 @@ _winapi_CreateProcess_impl(PyObject *module, Py_UNICODE *application_name,
         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
 
-    Py_XDECREF(environment);
+    if (!result) {
+        PyErr_SetFromWindowsErr(GetLastError());
+        goto cleanup;
+    }
 
-    if (! result)
-        return PyErr_SetFromWindowsErr(GetLastError());
+    ret = Py_BuildValue("NNkk",
+                        HANDLE_TO_PYNUM(pi.hProcess),
+                        HANDLE_TO_PYNUM(pi.hThread),
+                        pi.dwProcessId,
+                        pi.dwThreadId);
 
-    return Py_BuildValue("NNkk",
-                         HANDLE_TO_PYNUM(pi.hProcess),
-                         HANDLE_TO_PYNUM(pi.hThread),
-                         pi.dwProcessId,
-                         pi.dwThreadId);
+cleanup:
+    Py_XDECREF(environment);
+    freeattributelist(&attribute_list);
+
+    return ret;
 }
 
 /*[clinic input]
@@ -1489,7 +1661,6 @@ _winapi_WriteFile_impl(PyObject *module, HANDLE handle, PyObject *buffer,
     return Py_BuildValue("II", written, err);
 }
 
-
 /*[clinic input]
 _winapi.GetACP
 
@@ -1503,6 +1674,30 @@ _winapi_GetACP_impl(PyObject *module)
     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 @@ static PyMethodDef winapi_functions[] = {
     _WINAPI_WAITFORSINGLEOBJECT_METHODDEF
     _WINAPI_WRITEFILE_METHODDEF
     _WINAPI_GETACP_METHODDEF
+    _WINAPI_GETFILETYPE_METHODDEF
     {NULL, NULL}
 };
 
@@ -1623,6 +1819,12 @@ PyInit__winapi(void)
     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 80edbf58a20..b14f087732e 100644
--- a/Modules/clinic/_winapi.c.h
+++ b/Modules/clinic/_winapi.c.h
@@ -907,4 +907,38 @@ _winapi_GetACP(PyObject *module, PyObject *Py_UNUSED(ignored))
 {
     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]*/



More information about the Python-checkins mailing list