os.pipe should return inheritable descriptors (Windows)

[castironpi]

BPO	4708
Nosy	@vstinner, @tiran, @tjguk
Files	os_pipe_test.py: test showing subprocess inheriting handle inheritable_pipes.diff: patch to posixmodule.c including test case and documentation updates.

^{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:

assignee = None
closed_at = <Date 2014-02-13.13:52:42.906>
created_at = <Date 2008-12-21.02:15:24.191>
labels = ['type-bug', 'library', 'OS-windows']
title = 'os.pipe should return inheritable descriptors (Windows)'
updated_at = <Date 2014-02-13.13:52:42.905>
user = 'https://bugs.python.org/castironpi'

bugs.python.org fields:

activity = <Date 2014-02-13.13:52:42.905>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2014-02-13.13:52:42.906>
closer = 'vstinner'
components = ['Library (Lib)', 'Windows']
creation = <Date 2008-12-21.02:15:24.191>
creator = 'castironpi'
dependencies = []
files = ['12408', '12460']
hgrepos = []
issue_num = 4708
keywords = ['patch']
message_count = 10.0
messages = ['78136', '78166', '78321', '79844', '192660', '192733', '192736', '193637', '194775', '211156']
nosy_count = 8.0
nosy_names = ['ggenellina', 'vstinner', 'christian.heimes', 'tim.golden', 'jnoller', 'castironpi', 'Daniel.Goertzen', 'sbt']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue4708'
versions = ['Python 3.4']

[castironpi]

os.pipe should return inheritable descriptors on Windows.

Patch below, test attached. New pipe() returns descriptors, which
cannot be inherited. However, their permissions are set correctly, so
msvcrt.get_osfhandle and msvcrt.open_osfhandle can be used to obtain an
inheritable handle.

Docs should contain a note to the effect. 'On Windows, use
msvcrt.get_osfhandle to obtain a handle to the descriptor which can be
inherited. In a subprocess, use msvcrt.open_osfhandle to obtain a new
corresponding descriptor.'

--- posixmodule_orig.c  2008-12-20 20:01:38.296875000 -0600
+++ posixmodule_new.c   2008-12-20 20:01:54.359375000 -0600
@@ -6481,8 +6481,12 @@
        HANDLE read, write;
        int read_fd, write_fd;
        BOOL ok;
+       SECURITY_ATTRIBUTES sAttribs;
        Py_BEGIN_ALLOW_THREADS
-       ok = CreatePipe(&read, &write, NULL, 0);
+       sAttribs.nLength = sizeof( sAttribs );
+       sAttribs.lpSecurityDescriptor = NULL;
+       sAttribs.bInheritHandle = TRUE;
+       ok = CreatePipe(&read, &write, &sAttribs, 0);
        Py_END_ALLOW_THREADS
        if (!ok)
                return win32_error("CreatePipe", NULL);

[ggenellina]

From the thread in c.l.p:

Pros (of changing os.pipe() to return inheritable pipes):

• as it isn't explicitely documented whether os.pipe() returns
  inheritable pipes or not, both versions are "right" according to the
  documentation.

• if someone relies on pipes being non-inheritable on Windows (why?),
  this is undocumented behaviour, and Python has the right to change it.

• would improve POSIX compatibility, it mimics what os.pipe()
  does on those OS

• inheritable pipes are less surprising for guys coming from other OS

• inheritable pipes are a lot more useful than non-inheritable ones
  when doing IPC (probably its main usage).

Cons:

• os.pipe has behaved that way since long time ago.

• some programs *might* break, if they relied on pipes being
  non-inheritable on Windows, even if that was undocumented behaviour.

[ggenellina]

Patch to posixmodule.c including test case and documentation updates.
Note: I've only run the tests on Windows.

[castironpi]

This is currently accomplished in 'multiprocessing.forking' with a
'duplicate' function.

Use (line #213):
rfd, wfd = os.pipe()

\# get handle for read end of the pipe and make it inheritable
rhandle = duplicate(msvcrt.get_osfhandle(rfd), inheritable=True)

Definition (line #192).

Should it be included in the public interface and documented, or perhaps
a public entry point to it made?

[tiran]

Victor, this fits nicely with your recent PEP.

[sbt]

• would improve POSIX compatibility, it mimics what os.pipe()
  does on those OS

I disagree.

On Windows fds can only be inherited if you start processes using the spanwn*() family of functions. If you start them using CreateProcess() then the underlying *handles* are inherited, but the *fds* are not.

In Python 2, os.spawn*() used spawn*(), so making os.pipe() return inheritable fds would have made some sense. But in Python 3 os.spawn*() is implemented using subprocess/CreateProcess so fds will NOT be inherited (even if the wrapped handles are).

Note that subprocess *does* know how to redirect the standard streams to fds returned by os.pipe().

So for Python 3 I don't think there is any point in changing things.

[sbt]

Oops. I confused os.popen() with os.spawn*(). os.spawnv() IS still implemented using spawnv() in Python 3.

[vstinner]

Changing the default inheritance value of os.pipe() is not acceptable because it would break backward compatibility.

Giving access to Windows extra parameter is nice, but we have to find a way to propose a portable API. That's what I'm trying to do with the PEP-446 (and it's previous version, the PEP-433).

[vstinner]

os.pipe() creates non-inheritable pipes on Windows, whereas it creates inheritable pipes on UNIX. IMO the reason is an implementation artifact: os.pipe() calls CreatePipe() on Windows (native API), whereas it calls pipe() on UNIX (POSIX API). The call to CreatePipe() was added in Python in 1994, before the introduction of pipe() in the POSIX API in Windows 98.

[vstinner]

The PEP-446 has been implemented in Python 3.4 and all file descriptors and sockets are now created non-inheritable by default. Use os.set_inheritable() to make the pipe fds inheritable.