Windows implementation of `set_close_exec` will not work

[zooba]

The Windows implementation of set_close_exec https://github.com/tornadoweb/tornado/blob/master/tornado/platform/windows.py currently can never work because SetHandleInformation needs an OS handle, but everywhere this function is called it is given a file descriptor.

Firstly, I believe that by default all handles are non-inheritable. But I haven't checked everywhere you're creating them though - certainly the returned pipes from os.pipe are explicitly noninheritable.

Most likely you should just use os.set_inheritable on all platforms, which does what you need. It was added in Python 3.4, so once 2.7 is no longer a concern this is probably the easiest way forward. It knows how to convert FDs to handles on Windows.

Finally, I came across this because I'm trying to bring up a Python environment on Windows that doesn't include ctypes, and this showed up on my list of "concerns". I'll patch my own to use the builtin function, but figured I'd mention that the current code doesn't work.

[bdarnell]

Firstly, I believe that by default all handles are non-inheritable.

That's my understanding as well, so in theory we'd be OK with a no-op on windows.

Most likely you should just use os.set_inheritable on all platforms, which does what you need. It was added in Python 3.4, so once 2.7 is no longer a concern this is probably the easiest way forward.

Excellent. We have dropped support for 2.7 (our minimum python version is now 3.5) so we can just rip this out and use the os function.

Or, given this change in python 3.4, maybe we can rip all this out without replacing it. Just need to verify that all the times we're using this python is doing the right thing under the covers.

[zooba]

Just need to verify that all the times we're using this python is doing the right thing under the covers.

If it's not, that's a bug in Python. But since you're still supporting versions that won't ever be fixed, you're probably best to call set_inheritable anyway. (Though it's clearly not going to make anything worse on Windows if you skip it there ;) )

[zooba]

Oh, I should add that os.set_inheritable doesn't work on sockets on Windows, so quite likely you just want to rip it out completely.