New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
os.dup() fails for standard streams on Windows 7 #81730
Comments
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 bpo-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 bpo-37267 for input. |
This error didn't originate from the C dup() call, since that would be based on errno, like 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." Maybe we should add an inheritable paramater to os.dup() in Python 3.8? |
That's not necessarily correct. For example, if stdout is a pipe or disk file:
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. |
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. |
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.
For os.dup it says
The standard handles aren't relevant. Also, under "inheritance of file descriptors", it says:
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:
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!
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 :) |
This is marked as a release blocker. The last beta is scheduled for Monday. Please decide how to proceed ASAP. |
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: 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:
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 :) |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: