os.dup(fd) generates an OSError for standard streams (0: stdin, 1: stdout, 2: stderr) on Windows 7.  Works as expected on Windows 10.

Working backwards we found the issue first appears in Python 3.7.4rc1 and 3.8.0b2 on Windows 7.  Earlier releases work as expected.

>>> import os
>>> os.dup(1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OSError: [WinError 87] The parameter is incorrect
>>>

This seems likely to have been caused by the fixes for #37267, which fixes an issue with os.dup leaving character streams inheritable (when the documentation specifies that the result must be non-inheritable).

The code originally didn't try to make the descriptor non-inheritable because someone believed it wasn't allowed for character files, and the subsequent patch comments say "That was a mistake". Is it possible it wasn't allowed on Windows 7, and is allowed on Windows 10?

I'm nosying the folks from #37267 for input.

> OSError: [WinError 87] The parameter is incorrect

This error didn't originate from the C dup() call, since that would be based on errno, like `OSError: [Errno 9] Bad file descriptor`. It must come from _Py_set_inheritable. The only WINAPI call there is SetHandleInformation [1], which is documented to support "console input buffer" and "console screen buffer" handles, though it may be that the documentation is wrong.

Try the following code:

    >>> import os, msvcrt
    >>> msvcrt.get_osfhandle(0)
    84
    >>> os.set_handle_inheritable(msvcrt.get_osfhandle(0), False)

Prior to Windows 8, a console handle is tagged by setting the lower 2 bits (e.g. 3, 7, 11). The system uses this tag to direct calls to special console functions that route requests over the console LPC port. Thus, in the domain of File handles, setting the lower 2 bits is reserved to tag console handles. This should also be true in Windows 8+, even though console handle tagging is no longer used (now they're kernel handles for the ConDrv device). 

The above test will confirm my suspicion, but it looks some time around Windows XP/2003, Microsoft removed the internal [Get|Set]ConsoleHandleInformation functions that used to implement [Get|Set]HandleInformation for console handles, and the people responsible for the change failed to update the docs. Since the internal DuplicateConsoleHandle function wasn't removed, dup() itself still succeeds.

I suggest that _Py_set_inheritable should handle this case. If the call fails with ERROR_INVALID_PARAMETER, and the two tag bits of the handle are set, the handle is possibly (not necessarily) a console handle. In this case, if GetFileType is FILE_TYPE_CHARACTER, then it's a console handle, and the error should be ignored.

[1] https://docs.microsoft.com/en-us/windows/win32/api/handleapi/nf-handleapi-sethandleinformation

Results with Python 3.7.4 on Windows 7

>>> import os, msvcrt
>>> msvcrt.get_osfhandle(0)
15
>>> os.set_handle_inheritable(msvcrt.get_osfhandle(0), False)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OSError: [WinError 87] The parameter is incorrect
>>>

Oh, os.dup() doc says: "On Windows, when duplicating a standard stream (0: stdin, 1: stdout, 2: stderr), the new file descriptor is inheritable."
https://docs.python.org/dev/library/os.html

Maybe we should add an inheritable paramater to os.dup() in Python 3.8?

> os.dup() doc says: "On Windows, when duplicating a standard stream 
> (0: stdin, 1: stdout, 2: stderr), the new file descriptor is 
> inheritable."

That's not necessarily correct. For example, if stdout is a pipe or disk file:

    C:\>python -V
    Python 3.7.3
    
    C:\>python -c "import os; fd = os.dup(1); print(os.get_inheritable(fd))" | more
    False

    C:\>python -c "import os; fd = os.dup(1); print(os.get_inheritable(fd))" > stdout.txt

    C:\>type stdout.txt
    False

What 3.7.3 does is to skip calling _Py_set_inheritable for all files of type FILE_TYPE_CHAR, which is wrong for character devices in general, such as NUL and serial ports. NUL is a common target for standard I/O redirection.

This is marked as release blocker but since there is no movement on the possible solution, I'm releasing 3.80b3 without a fix. Please resolve this by b4, I will block the last beta on this issue.

Honestly, I'm not sure of what is the right solution for this issue.

> What 3.7.3 does is to skip calling _Py_set_inheritable for all files of type FILE_TYPE_CHAR

It sounds like we should probably revert to the middle ground, and skip _raising the error_ if _Py_set_inheritable for files of type FILE_TYPE_CHAR.

As Victor pointed out, the docs already call out the special case in Windows. Since this applies to Windows 7, which will be EOL only a couple months after 3.8's release, I think it's fine to have the slightly less secure behavior and just suppress the error.

> It sounds like we should probably revert to the middle ground, and 
> skip _raising the error_ if _Py_set_inheritable for files of type
> FILE_TYPE_CHAR.

The problem is just console pseudohandles in Windows 7 and earlier.  Maybe it should just check for this case before calling SetHandleInformation. For example:

    /* This check can be removed once support for Windows 7 ends. */
    #define CONSOLE_PSEUDOHANDLE(handle) (((ULONG_PTR)(handle) & 0x3) == 0x3 && \
            GetFileType(handle) == FILE_TYPE_CHAR)

    if (!CONSOLE_PSEUDOHANDLE(handle) &&
        !SetHandleInformation(handle, HANDLE_FLAG_INHERIT, flags)) {
        if (raise)
            PyErr_SetFromWindowsErr(0);
        return -1;
    }

We have similar Python code in subprocess Popen._filter_handle_list, which filters out console pseudohandles because they don't work with PROC_THREAD_ATTRIBUTE_HANDLE_LIST.

A similar check could be added to os_set_handle_inheritable_impl in Modules/posixmodule.c.

> As Victor pointed out, the docs already call out the special case in 
> Windows. 

For os.dup it says

    On Windows, when duplicating a standard stream (0: stdin, 1: 
    stdout, 2: stderr), the new file descriptor is inheritable.

The standard handles aren't relevant.

Also, under "inheritance of file descriptors", it says:

    On Windows, non-inheritable handles and file descriptors are closed 
    in child processes, except for standard streams (file descriptors 0,
    1 and 2: stdin, stdout and stderr), which are always inherited.

The standard handles aren't always inherited. If bInheritHandles is TRUE (i.e. close_fds=False), a standard handle has to be iheritable for it to be inherited. 

For example, in Windows 10, make stderr (a ConDrv console handle) non-inheritable and create a child process with close_fds=False:

    >>> os.set_handle_inheritable(msvcrt.get_osfhandle(2), False)
    >>> cmd = 'import sys; print(sys.stderr)'
    >>> subprocess.call(f'python.exe -c "{cmd}"', close_fds=False)
    None
    0

In Windows 7 and earlier, it seems that inheritable console pseudohandles are always inherited, regardless of bInheritHandles -- as long as the child process attaches to the parent's console. SetHandleInformation isn't supported for console pseudohandles, but the inheritable flag is still set or cleared when a console pseudohandle is created via CreateConsoleScreenBuffer, CreateFileW (routed to OpenConsoleW), and DuplicateHandle (routed to DuplicateConsoleHandle).

It's worth mentioning that the system sometimes duplicates (not inherits) standard handles to a child process. A simple example in Windows 10 would be subprocess.call('python.exe'). 

All of the follow requirements must be satisfied for CreateProcessW to duplicate a standard handle to a child:

* it's not a console pseudohandle (e.g. it's a ConDrv console handle in 
  Windows 8+, or handle for the NUL device, a pipe, or disk file)
* the target executable is a console application (e.g. python.exe, not 
  pythonw.exe)
* handle inheritance is disabled (i.e. bInheritHandles is FALSE)
* the startup-info standard handles aren't used (i.e. 
  STARTF_USESTDHANDLES isn't set)
* the call isn't flagged to execute without a console or to allocate a
  new console (i.e. no DETACHED_PROCESS, CREATE_NEW_CONSOLE, or 
  CREATE_NO_WINDOW)

In this case, CreateProcessW also has to update the handle value in the child since generally the duplicate has a new value. With inheritance, in contrast, the handle value is the same in the parent and child.

Thanks Zackery for the patch!

> The problem is just console pseudohandles in Windows 7 and earlier.

Should we add a version check as well [1]? I'm not totally comfortable with bitmasking a handle here, but if we only do this additional check on Win7 then I'm okay with it.

And if we add the check, I'd rather this be a function than a macro. It's getting too complicated to read :)

[1]: https://docs.microsoft.com/en-us/windows/win32/api/versionhelpers/nf-versionhelpers-iswindows8orgreater

This is marked as a release blocker. The last beta is scheduled for Monday. Please decide how to proceed ASAP.

> Should we add a version check as well [1]? I'm not totally comfortable 
> with bitmasking a handle here, but if we only do this additional 
> check on Win7 then I'm okay with it.

A version check should not be necessary. For file objects, setting both handle tag bits has always been a reserved signature for console pseudohandles. Here's the definition in the published console code:

https://github.com/microsoft/terminal/blob/b726a3d05d35928becc8313f6ac5536c9f503645/dep/Console/winconp.h#L460

It's mostly vestigial in Windows 8+ due to the addition of the ConDrv console device. Code that routed calls to LPC-based implementations when passed a console pseudohandle has all been removed. The only remaining use I know of is in the console attach code. If a process is started with STARTF_USESTDHANDLES, then AllocConsole() will only replace handles that are NULL or console pseudohandles. The docs for GetStdHandle explain this:

    Attach/detach behavior

    When attaching to a new console, standard handles are always replaced
    with console handles unless STARTF_USESTDHANDLES was specified during
    process creation.

    If the existing value of the standard handle is NULL, or the existing
    value of the standard handle looks like a console pseudohandle, the
    handle is replaced with a console handle.

    When a parent uses both CREATE_NEW_CONSOLE and STARTF_USESTDHANDLES to
    create a console process, standard handles will not be replaced unless
    the existing value of the standard handle is NULL or a console
    pseudohandle.

I confirmed that this is still relevant in Windows 10 by testing an inherited pipe handle in stdout and stderr and manually setting the tag bits in the hStdError handle value in STARTUPINFO. The console attach code preserved the regular handle in stdout but replaced the console pseudohandle in stderr. So the API is still reserving these tag bits in file handles, at least in one place, which means they're still reserved in general, even though setting them will no longer break a ReadFile call, for one example, like it would in Windows 7 (in which the API would route the call to the internal ReadConsole implementation). 

We've been relying on checking the console pseudohandle tag bits without a version check in subprocess.Popen._filter_handle_list for a while now. Console pseudohandles have to be removed from the handle list, else CreateProcessW will fail.

Thanks, Eryk. In that case, I'll merge the PR.

Since Python 3.9 will not support Windows 7, we can remove it from master as soon as the backports are done :)

New changeset 5be666010e4df65dc4d831435cc92340ea369f94 by Steve Dower (Zackery Spytz) in branch 'master':
bpo-37549: os.dup() fails for standard streams on Windows 7 (GH-15389)
https://github.com/python/cpython/commit/5be666010e4df65dc4d831435cc92340ea369f94

New changeset f8dc3e85ab01e1b0345738670dcb3993da7ec436 by Miss Islington (bot) in branch '3.7':
bpo-37549: os.dup() fails for standard streams on Windows 7 (GH-15389)
https://github.com/python/cpython/commit/f8dc3e85ab01e1b0345738670dcb3993da7ec436

New changeset 3921d12174c1998d9df7a08d036a7fef2d587a64 by Miss Islington (bot) in branch '3.8':
bpo-37549: os.dup() fails for standard streams on Windows 7 (GH-15389)
https://github.com/python/cpython/commit/3921d12174c1998d9df7a08d036a7fef2d587a64