Created on 2013-01-03 15:23 by haypo, last changed 2013-04-09 22:15 by haypo. This issue is now closed.
|open_mode_e-2.patch||haypo, 2013-01-07 22:49||review|
|msg178949 - (view)||Author: STINNER Victor (haypo) *||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) *||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) *||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) *||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) *||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) *||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) *||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) *||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) *||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) *||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) *||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) *||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) *||Date: 2013-01-04 14:23|
|msg179027 - (view)||Author: STINNER Victor (haypo) *||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) *||Date: 2013-01-07 12:05|
Oh, my patch doesn't check fcntl() error code.
|msg179294 - (view)||Author: STINNER Victor (haypo) *||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) *||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) *||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) *||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) *||Date: 2013-04-09 22:15|
|2013-04-09 22:15:51||haypo||set||status: open -> closed|
superseder: Implementation of the PEP 433: Easier suppression of file descriptor inheritance
messages: + msg186459
|2013-01-13 00:06:25||haypo||set||messages: + msg179842|
|2013-01-09 23:43:11||haypo||set||messages: + msg179500|
|2013-01-08 23:07:15||haypo||set||messages: + msg179388|
|2013-01-08 00:00:47||haypo||set||files: - open_mode_e.patch|
messages: + msg179294
|2013-01-07 15:56:21||haypo||set||title: 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:50||haypo||set||messages: + msg179258|
|2013-01-07 12:04:54||haypo||set||title: Atomic open + close-and-exec -> Add "x" mode to open(): close-and-exec (O_CLOEXEC) / O_NOINHERIT|
keywords: + patch
messages: + msg179027
|2013-01-04 14:23:27||haypo||set||messages: + msg179024|
|2013-01-04 14:17:25||neologix||set||messages: + msg179022|
|2013-01-04 13:46:22||haypo||set||messages: + msg179021|
|2013-01-04 13:27:52||neologix||set||messages: + msg179020|
|2013-01-03 23:42:54||haypo||set||messages: + msg178999|
|2013-01-03 23:08:38||haypo||set||messages: + msg178994|
|2013-01-03 20:10:52||neologix||set||messages: + msg178984|
messages: + msg178982
|2013-01-03 16:25:29||christian.heimes||set||messages: + msg178959|
|2013-01-03 15:47:47||haypo||set||messages: + msg178957|
messages: + msg178953
|2013-01-03 15:30:04||haypo||set||messages: + msg178952|