Author eryksun
Recipients eryksun, izbyshev, njs, paul.moore, steve.dower, tim.golden, zach.ware
Date 2018-02-18.07:44:38
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1518939879.89.0.467229070634.issue32865@psf.upfronthosting.co.za>
In-reply-to
Content
> 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.
History
Date User Action Args
2018-02-18 07:44:39eryksunsetrecipients: + eryksun, paul.moore, tim.golden, njs, zach.ware, steve.dower, izbyshev
2018-02-18 07:44:39eryksunsetmessageid: <1518939879.89.0.467229070634.issue32865@psf.upfronthosting.co.za>
2018-02-18 07:44:39eryksunlinkissue32865 messages
2018-02-18 07:44:38eryksuncreate