classification
Title: Add "e" mode to open(): close-and-exec (O_CLOEXEC) / O_NOINHERIT
Type: Stage:
Components: Interpreter Core Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder: Implementation of the PEP 433: Easier suppression of file descriptor inheritance
View: 17036
Assigned To: Nosy List: Arfrever, alexey-smirnov, amaury.forgeotdarc, christian.heimes, haypo, neologix, pitrou, rosslagerwall, sbt
Priority: normal Keywords: patch

Created on 2013-01-03 15:23 by haypo, last changed 2013-04-09 22:15 by haypo. This issue is now closed.

Files
File name Uploaded Description Edit
open_mode_e-2.patch haypo, 2013-01-07 22:49 review
Messages (20)
msg178949 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-01-03 15:23
Recent version on different operating systems support opening a file with close-on-exec flag set immediatly (atomic). This feature fixes a race condition when the process calls execv() between open() and fcntl() (to set the FD_CLOEXEC flag to the newly opened file).

It would be nice to expose this feature in Python. The problem is the find a portable and safe way to expose the feature: neologix is against a best-effort function. For example, Linux kernel older than 2.6.22 simply ignores O_CLOEXEC flag (while the libc may expose it).

The feature looks to be supported by at least:

 * Linux kernel >= 2.6.23
 * FreeBSD 8+
 * Windows: _open(filename, _O_NOINHERIT). Is it supported by Windows XP and older versions? http://msdn.microsoft.com/en-us/library/z0kc8e3z.aspx

See also:

 * Issue #12760 (closed): This issue added an "x" mode to open() to create a file in exclusive mode
 * Issue #12103: "Document how to use open with os.O_CLOEXEC"
 * Issue #12105: It was proposed to add an "e" mode to open() for O_CLOEXEC
msg178952 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-01-03 15:30
> The problem is the find a portable and safe way to expose the feature

A solution is to add a "e" mode to open() which would raise a NotImplementedError if the platform is not known to support this feature. For example, if the OS is linux, we would check if the kernel version is at least 2.6.23, otherwise an exception would be raised.

The check (on the OS/version) would be done at the first call the function (if the "e" mode if used).

We already have such behaviour on other functions.
msg178953 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-01-03 15:32
You could do both: use the O_CLOEXEC flag and do a fcntl() call on POSIX. In my opinion it's enough to document that the x flag may be affected by a race condition issue on some operation systems.
msg178957 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-01-03 15:47
> You could do both: use the O_CLOEXEC flag and do a fcntl() call on POSIX

This is the best-effort option. It was already discussed and rejected in the issue #12760.

We had a similar discussion for the PEP 418 on monotonic clock. The final decision is not only provide time.monotonic() if the OS supports monotonic clock.

Using an exception, a developer can develop its own best-effort function:

try:
    fileobj = open(filename, "e")
except NotImplementedError:
    fileobj = open(filename, "r")
    # may also fail here if the fcntl module is missing
    import fcntl
    flags = fcntl.fcntl(self.socket, fcntl.F_GETFD)
    fcntl.fcntl(fileobj, fcntl.F_SETFD, flags | fcntl.FD_CLOEXEC)

We may expose the best-effort function somewhere else, but not in open() which must be atomic in my opinion.
msg178959 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-01-03 16:25
Do you mean #12105? I didn't know about the ticket before.

How about two options, like 'e' for guaranteed atomic CLOEXEC and 'E' for CLOEXEC with or without atomic ops? It's not much additional work and lowers the burden on the user.

It's going to be hard to get a list of platforms and versions where O_CLOEXEC is supported. We should go for a white list approach and only enable it on platforms that are either documented to support it or have been tested.
msg178982 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-01-03 20:02
> How about two options, like 'e' for guaranteed atomic CLOEXEC and 'E'
> for CLOEXEC with or without atomic ops?

Why would you want that? Best effort is sufficient.
Also, I'm not sure why "e".
msg178984 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2013-01-03 20:10
I don't comfortable exposing this.
The main reason is that this flag is really non-portable.
Having open() fail at runtime because the platform doesn't support it looks really wrong to me. And silently ignore it is even worse.
The 'x' flag was added because it is useful, on available on all platforms (I mean, even Windows has it). Here's, it's by definition Unix-specific, and even then, many Unices don't support it.
So I'm -1.
msg178994 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-01-03 23:08
> The feature looks to be supported by at least:
> * FreeBSD 8+

Hum, it looks like it was only added to FreeBSD 8.3:

http://www.freebsd.org/cgi/man.cgi?query=open&apropos=0&sektion=0&manpath=FreeBSD+8.3-RELEASE&arch=default&format=html

(O_CLOEXEC doesn't appear in FreeBSD 8.2 manual page)
msg178999 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-01-03 23:42
> Also, I'm not sure why "e".

The choice of the "e" letter comes from the GNU version of fopen(). Extract of fopen manual page on Linux:

       e (since glibc 2.7)
              Open the file with the O_CLOEXEC flag.  See open(2) for more information.

Oh, by the way: "e" mode is a best-effort approach. It uses fcntl() if O_CLOEXEC is not supported by the running kernel. fopen() doesn't check the kernel version, but pass O_CLOEXEC and check (once) if FD_CLOEXEC is set:

http://sourceware.org/git/?p=glibc.git;a=blob;f=libio/fileops.c;h=61b61b30d88d0fca9e6b20a2fe62a2406cc027cf;hb=HEAD#l319

It's interesting to know that the glibc chose the best-effort approach.

> The 'x' flag was added because it is useful, on available on all
> platforms (I mean, even Windows has it). Here's, it's by definition
> Unix-specific, ...

Windows provides O_NOINHERIT (_O_NOINHERIT) flag which has a similar purpose.

> ... and even then, many Unices don't support it.

Yes, but I bet that more and more OSes will support it :-) For example, it looks like O_CLOEXEC is part of the POSIX standard 2008:

http://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html
("O_CLOEXEC: If set, the FD_CLOEXEC flag for the new file descriptor shall be set.")

The O_CLOEXEC flag of Linux comes from QNX and BeOS which support it since many years (I don't know when, at least since 2004).

So O_CLOEXEC is not really specific to Linux.

--

For more information of the Linux support, this article of Ulrich Drepper contains many useful information:
http://udrepper.livejournal.com/20407.html
msg179020 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2013-01-04 13:27
> Windows provides O_NOINHERIT (_O_NOINHERIT) flag which has a similar purpose.
>
>> ... and even then, many Unices don't support it.
>
> Yes, but I bet that more and more OSes will support it :-) For example, it looks like O_CLOEXEC is part of the POSIX standard 2008:

Hum, OK, but many operating systems don't support it to this day.
So if we expose it and the underlying operating system doesn't support
it, do you want to fallback to fcntl (which wouldb't be atomic
anymore, but let's pretend the GIL protection is enough).

Also, I'm not sure exactly if this flag is useful enough to be
exposed: there are many flags that can be passed when opening a file:
O_DIRECT, O_SYNC, O_NONBLOCK, O_DSYNC...
Amusingly, Java exposes some of them (but not O_CLOEXEC):
http://docs.oracle.com/javase/7/docs/api/java/nio/file/StandardOpenOption.html
msg179021 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-01-04 13:46
> So if we expose it and the underlying operating system doesn't support
it, do you want to fallback to fcntl (which wouldb't be atomic
anymore, but let's pretend the GIL protection is enough).

At the beginning, I was convinced that the atomicity was important than the portability. But after having read that even fopen() uses a fallback, it is maybe much more convinient to have a fallback.

For example, it would be annoying that a program works on Linux 2.6.23, but not on Linux 2.6.22 whereas the atomicity is not a must-have. Said differently: the manual fallback described in msg178957 now seems annoying to me :-)

So let's say that a fallback is preferable to improve the portability, but that open(name, "e") would still fail with NotImplementedError if O_CLOEXEC, O_NOINHERIT and fcntl(FD_CLOEXEC) are all missing on a platform. I guess that all platform provide at least one flag/function.

You already implemented a similar fallback for subprocess: use pipe2(O_CLOEXEC) if available, or fallback to pipe()+fcntl(FD_CLOEXEC).

> I'm not sure exactly if this flag is useful enough to be exposed

I would like to benefit of the atomicity feature of O_CLOEXEC, without having to implement myself all the tricky things to check if the platform supports it or not.

O_CLOEXEC solves for example a race condition in tempfile._mkstemp_inner():

            fd = _os.open(file, flags, 0o600)
            _set_cloexec(fd)
msg179022 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2013-01-04 14:17
> O_CLOEXEC solves for example a race condition in tempfile._mkstemp_inner():
>
>             fd = _os.open(file, flags, 0o600)
>             _set_cloexec(fd)

Hum...
"""
diff --git a/Lib/tempfile.py b/Lib/tempfile.py
--- a/Lib/tempfile.py
+++ b/Lib/tempfile.py
@@ -57,6 +57,8 @@
 _allocate_lock = _thread.allocate_lock

 _text_openflags = _os.O_RDWR | _os.O_CREAT | _os.O_EXCL
+if hasattr(_os, 'O_CLOEXEC'):
+    _text_openflags |= _os.O_CLOEXEC
 if hasattr(_os, 'O_NOINHERIT'):
     _text_openflags |= _os.O_NOINHERIT
 if hasattr(_os, 'O_NOFOLLOW'):
"""

We should probably add this to 3.3 and default (IIRC O_CLOEXEC was
added to the os module in 3.3).
Also, I think O_NOFOLLOW is useless: if the file is created
(O_EXCL|O_CREAT), then by definition it's not a symlink (juste check
glibc's mkstemp() implementation, and it only passes O_CREAT|O_EXCL).
msg179024 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-01-04 14:23
> We should probably add this to 3.3 and default (IIRC O_CLOEXEC was
added to the os module in 3.3).

I created the issue #16860. I just realized that tempfile doesn't use open() but os.open(), so this issue help to fix #16860.
msg179027 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-01-04 14:30
Here is a work-in-progress patch to test my idea: add "e" flag to open().

Limitations/TODO:

 * The unit test doesn't work on Windows (it requires fcntl)
 * "e" mode and the opener argument are exclusive: if O_CLOEXEC and O_NOINHERINT are missing, I don't see how the opener can be aware of the "e" flag. Or should we call fcntl(F_SETFD, flags|FD_CLOEXEC) after the opener? It would be strange: the opener is supposed to be the only one to decide how the file is opened, isn't it?
 * NotImplementedError is raised if O_CLOEXEC, O_NOINHERIT and fcntl() are missing

I only tested my patch on Linux (3.6).
msg179258 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-01-07 12:05
Oh, my patch doesn't check fcntl() error code.
msg179294 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-01-07 22:49
Updated patch. I tested it on Linux Fedora (3.6), Linux Debian (3.0), Windows 7, FreeBSD 8.2 (don't support O_CLOEXEC), OpenIndiana (oi_148, SunOS 5.11), OpenBSD 4.9, OpenBSD 5.2, Mac OS X 10.8. test_builtin pass on all these platforms... except OpenBSD 4.9.

The test fails on OpenBSD 4.9 for an unknown reason. fcntl(FD_CLOEXEC) + exec() works, but fcntl(FD_CLOSEXEC) + fork() + exec() fails. It looks like an OS bug, I don't see what Python can do for this case :-/ The "e" mode does correctly set FD_CLOEXEC, but FD_CLOEXEC doesn't work as expected. It's really an OS bug, because test_builtin pass on OpenBSD 5.2.

I don't have other OSes to check if another platform doesn't support "e" mode (a platform where NotImplementedError would be raised).
msg179388 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-01-08 23:07
According to the following script, FD_CLOEXEC is available on:
 * Solaris 8
 * HP-UX 11
 * Unicos 9.0
 * Digital UNIX 4.0
 * SunOS 4.1.1_U1
 * IRIX 6.5
 * Linux 2.0 and Linux 2.4 with glibc 2.2.3
 * AIX 4.2
(Versions are tested versions, it doesn't mean that the flag didn't exist before)

http://sam.nipl.net/ssh-fast/fsh/fshcompat.py

--

Note: On Windows, it's possible to implement fcntl(fd, F_SETFD, FD_CLOEXEC) using: SetHandleInformation(fd, HANDLE_FLAG_INHERIT, 1). See for example:
https://github.com/tav/pylibs/blob/master/tornado/win32_support.py#L36

I don't know if it's useful since it's possible to set the flag directly when the file is opened (using O_NOINHERIT).
msg179500 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-01-09 23:43
There is another way to set close-on-exec flag on a file descriptor: "ioctl(fd, FIOCLEX, 0);" (and "ioctl(fd, FIONCLEX, 0);" to unset the flag). It is interesting because it avoids the need to get the flags before setting new flags (old | FD_CLOEXEC): 1 syscall instead of 2.

ioctl(fd, FIOCLEX) is available on at least: Linux, Mac OS X, QNX, NetBSD, OpenBSD, FreeBSD. I don't know if it's available in old versions of these operating systems. (It is *not* supported by Interix.)
msg179842 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-01-13 00:06
I created the PEP 433 which proposes a more global change to set the close-on-exec flag.
msg186459 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-04-09 22:15
The PEP 433 and its issue #17036 replaces this issue, they are more generic.
History
Date User Action Args
2013-04-09 22:15:51hayposetstatus: open -> closed
superseder: Implementation of the PEP 433: Easier suppression of file descriptor inheritance
resolution: fixed
messages: + msg186459
2013-01-13 08:22:24Arfreversetnosy: + Arfrever
2013-01-13 00:06:25hayposetmessages: + msg179842
2013-01-09 23:43:11hayposetmessages: + msg179500
2013-01-08 23:07:15hayposetmessages: + msg179388
2013-01-08 00:00:47hayposetfiles: - open_mode_e.patch
2013-01-07 22:50:00hayposetfiles: + open_mode_e-2.patch

messages: + msg179294
2013-01-07 15:56:21hayposettitle: Add "x" mode to open(): close-and-exec (O_CLOEXEC) / O_NOINHERIT -> Add "e" mode to open(): close-and-exec (O_CLOEXEC) / O_NOINHERIT
2013-01-07 12:05:50hayposetmessages: + msg179258
2013-01-07 12:04:54hayposettitle: Atomic open + close-and-exec -> Add "x" mode to open(): close-and-exec (O_CLOEXEC) / O_NOINHERIT
2013-01-04 14:30:25hayposetfiles: + open_mode_e.patch
keywords: + patch
messages: + msg179027
2013-01-04 14:23:27hayposetmessages: + msg179024
2013-01-04 14:17:25neologixsetmessages: + msg179022
2013-01-04 13:46:22hayposetmessages: + msg179021
2013-01-04 13:27:52neologixsetmessages: + msg179020
2013-01-03 23:42:54hayposetmessages: + msg178999
2013-01-03 23:08:38hayposetmessages: + msg178994
2013-01-03 20:10:52neologixsetmessages: + msg178984
2013-01-03 20:02:08pitrousetnosy: + pitrou
messages: + msg178982
2013-01-03 19:30:53rosslagerwallsetnosy: + rosslagerwall
2013-01-03 16:25:29christian.heimessetmessages: + msg178959
2013-01-03 15:47:47hayposetmessages: + msg178957
2013-01-03 15:32:58christian.heimessetnosy: + christian.heimes
messages: + msg178953
2013-01-03 15:30:04hayposetmessages: + msg178952
2013-01-03 15:23:26haypocreate