Eryk Sun <eryk...@gmail.com> added the comment:

> The first is a bug in os.pipe (creation of an inheritable descriptor
> with non-inheritable underlying handle). This can be fixed by using
> _open_osfhandle() correctly.

This is the only issue to be addressed here, and it's an easy fix. The problem 
also occurs in Modules/_io/winconsoleio.c. 

I digressed on the problems with _Py_get_inheritable and _Py_set_inheritable. 
Such problems need to be addressed in a separate issue.

> Does an FD get flagged as a pipe by calling GetFileType in 
> _open_osfhandle?

Yes, it calls GetFileType to check for a bad handle (FILE_TYPE_UNKNOWN) and 
also to flag whether the FD type is FDEV (FILE_TYPE_CHAR) or FPIPE 
(FILE_TYPE_PIPE). See lowio\osfinfo.cpp in the CRT source code.

> Also, it's totally unclear to me how something like GetFileType 
> can interfere with I/O and cause a deadlock.

You're right to question this. I couldn't reproduce this problem in Windows 10 
using a local, anonymous pipe. GetFileType calls NtQueryVolumeInformationFile 
[1] to get the FileFsDeviceInformation for the referenced File. For local 
devices, the I/O Manager can simply copy the requested information from the 
associated Device object (e.g. \Device\NamedPipe); it doesn't even need to call 
the device driver with an IRP_MJ_QUERY_VOLUME_INFORMATION request. Maybe older 
implementations of the I/O Manager always waited to acquire the File's internal 
lock when opened in synchronous mode.

Whether this is (or ever was) a valid reason, it's the reason given for 
skipping the call in the CRT function initialize_inherited_file_handles_nolock 
(lowio\ioinit.cpp). There's nothing we can do about it. The CRT will not 
validate an inherited pipe FD at startup. Also, even this validation can't help 
if the same handle just happens to be assigned to another File at startup.

[1]: 
https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/content/ntifs/nf-ntifs-ntqueryvolumeinformationfile

> Both issues cause out-of-sync which is usually hidden and harmless 
> because people prefer subprocess.Popen() 

Python has to respect file descriptors as a common resource in the process. 
Other libraries or an embedding application may be using the same CRT. It's not 
simply a matter of what's commonly used in pure-Python code. That said, 
os.system() is still commonly used, and the CRT implements _wsystem via 
common_spawnv<wchar_t>.

> one nasty thought is to poke msvcrt internal structures like in _PyVerify_fd 

That's an ugly option, but it may be the best (or only possible) approach until 
MSVC adds a public function to modify the heritability of an FD.

On a related note, in looking at places where FDs are made non-inheritable, I 
see it's also used in _Py_fopen_obj. This function could be modified to use 
MSVC's "N" flag to ensure the underlying FD is opened with _O_NOINHERIT. glibc 
has a similar "e" flag for O_CLOEXEC.

----------

_______________________________________
Python tracker <rep...@bugs.python.org>
<https://bugs.python.org/issue32865>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to