New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
open() does not able to set flags, such as O_CLOEXEC #56314
Comments
In Python3 I'm not able to use open('xxx', 're') as it does not call fopen() anymore. What about extra flags like 'e'=O_CLOEXEC? P.S. P.P.S |
These "needed" flags are actually nonstandard extensions of the libc fopen() function on some platforms. In python3, one can still use fcntl(f.fileno(), FD_SET, FD_CLOEXEC), but a "flags" parameter would be more convenient. |
Note that it's not atomic. |
Here's a patch adding O_CLOEXEC to the os module, with test. This patch makes it possible to open and set a FD CLOEXEC atomically. |
Using spawn_python() to check that os.O_CLOEXEC flag is correctly set seems overkill. Why not just testing fcntl.fcntl(f.fileno(), fcntl.F_GETFL) & FD_CLOEXEC)? I don't think that there are OSes with O_CLOEXEC but without fcntl(F_GETFL).
I agree. open() documentation may explain the os.fdopen(os.open()) "trick" to use low-level options like O_SYNC or O_CLOEXEC. |
+1 for the patch! python2.7: open(filename, 're') |
Because I couldn't find a place where the CLOEXEC flag was fully
OK.
Why not, but I leave it to someone more comfortable with documentation |
Why not to implement 'e' letter in py3k ? In systems where O_CLOEXEC is not supported in open(), flag should be set non-atomically using fcntl. To exclude races (in concurrent threads), this two ops should be done under lock (GIL?) |
Having an atomic or non-atomic behaviour depending on the OS is not a |
That won't work, because open(), like other slow syscalls, is called without the GIL held. Furthermore, it wouldn't be atomic anyway (imagine a fork is done from a C extension without the GIL held). |
@charles-francois.natali: Your patch is ok, you can commit it into 3.1, 3.2, 3.3. But you may wait after 3.2.1. @georg Brandl: Should we wait after Python 3.2.1 for this issue? |
Issue bpo-12103 should be fine for this idea. |
Oh, by the way: it would also be nice to add os.O_CLOEXEC to Python 2.7. For example, tempfile._mkstemp_inner() uses: |
One moment -- adding a new value to the os module looks like a new feature to me. Is there any convincing reason why this needs to go to 3.2? (And it most definitely shouldn't go to 3.1.) |
No reason. I think this is definitely 3.3 material. |
New changeset f1c544245eab by Charles-François Natali in branch 'default': |
I've committed the patch to 3.3. |
And apparently some buildbot doesn't like it: ====================================================================== Traceback (most recent call last):
File "/var/lib/buildslave/3.x.murray-gentoo-wide/build/Lib/test/test_posix.py", line 315, in test_oscloexec
self.assertTrue(fcntl.fcntl(fd, fcntl.F_GETFD) & fcntl.FD_CLOEXEC)
AssertionError: 0 is not true |
Python doesn't suppose atomic open+CLOEXEC anymore, I consider this as a Can we add new features to old releases? |
It has never been documented (nor supported) so, no, I wouldn't consider
Which means it's certainly unimportant.
Well, you already know the answer, don't you? :) |
Linux-2.6.22-vs2.2.0.7-gentoo-i686-Intel-R-_Xeon-TM-_CPU_2.80GHz-with-gentoo-2.0.1 little-endian O_CLOEXEC support was added to Linux 2.6.23: this means that the libc defines it while the kernel doesn't support it (it's probably defined as 0). |
New changeset bff9265d677d by Victor Stinner in branch 'default': |
I like this solution, but I don't know how to test that the kernel doesn't support O_CLOEXEC. My commit bff9265d677d will tell use the value of O_CLOEXEC on the "Linux-2.6.22-vs2.2.0.7-gentoo-i686-Intel-R-_Xeon-TM-_CPU_2.80GHz-with-gentoo-2.0.1 little-endian" buildbot. |
If O_CLOEXEC is a #defined constant in the glibc, it can't be different |
Story of O_CLOEXEC in the GNU libc, by Ulrich Drepper: "Secure File Descriptor Handling" --
Hum, if I patch tempfile._mkstemp_inner() to use os.O_CLOEXEC if available, whereas this flag has no effect, the function will be less secure on Linux 2.6.22 (if the GNU libc exposes O_CLOEXEC). Check O_CLOEXEC is configure is not safe if the host compiling Python uses a different kernel that the host running Python (e.g. think of Linux distro: Python is compiled on a different computer). We need maybe a flag to indicate that O_CLOEXEC is supported by the running kernel or not. Or maybe a function to check it at least? -- O_CLOEXEC was added to Ruby and then removed because the flag is sometimes ignored: "because boron chkbuild test result says, An old linux kernel ignore O_CLOEXEC silently instead of return an error. It may lead to bring new security risk. So, we have to be pending it until finish to implement proper fallback logic." -- Read also the discussion about O_CLOEXEC on bugs-findutils mailing list: Their patch uses : static int
fd_is_cloexec (int fd)
{
const int flags = fcntl (fd, F_GETFD);
return (flags & FD_CLOEXEC) || (fcntl (fd, F_GETFL) & O_CLOEXEC);
}
static bool
o_cloexec_works (void)
{
bool result = false;
int fd = open ("/", O_RDONLY|O_CLOEXEC);
if (fd >= 0)
{
result = fd_is_cloexec (fd);
close (fd);
}
return result;
} -- Oh, there is also SOCK_CLOEXEC, which may be useful for bpo-5715. |
Another idea is to write a best-effort function to open a file with CLOEXEC flag:
Attached open_cloexec.py is an implementation. -- Usage of "CLOEXEC" in the Python standard library:
You may also add pipe_cloexec() to os, and socket_cloexec() to socket? |
Here you have: It's the same value on my Debian Sid. So we cannot test O_CLOEXEC value, we have to check the kernel version. |
The real issue is that the libc defines O_CLOEXEC, but kernels prior to 2.6.23 don't support it: instead of returning EINVAL, the socket syscall silently ignores the flag (don't know why I made the comment about this flag being defined to 0...). I personally don't like the idea of a best-effort O_CLOEXEC implementation: providing a O_CLOEXEC flag which is not atomic feels really wrong to me. |
This is a kernel bug, not a bug in the GNU libc (ask Ulrich if you are not sure ;-)). An host can have multiple kernel versions (and choose at boot time using GRUB/LILO/...), but it has usually only one C library. You cannot change the version of the libc depending on the kernel. If you really want to fix this problem, you will have to patch kernel < 2.6.23. Good luck! Or we can workaround kernel bugs providing a documentation and/or functions for that. |
I created the issue bpo-12158 for that. |
Kernels prior to 2.6.23 didn't know about the O_CLOEXEC flag: to catch this kind of problem, every syscall would have to check every bit when it's passed a combination of flags. This would be clumsy, error-prone and slow.
It's possible, but it's definitely a bad idea, because of such API mismatch. For example nothing prevents a syscall from being removed/modified from one kernel version to another. If your libc doesn't follow, you're up for trouble. |
@victor |
New changeset 9a16fa0c9548 by Victor Stinner in branch 'default': |
Checking the kernel version did the trick, the test now run fines on the buildbots. |
O_CLOEXEC is not linux-only. Windows has the same flag. In file-opening functions there is lpSecurityAttributes argument. And there is bInheritHandle member of corresponding structure. http://msdn.microsoft.com/en-us/library/aa379560(v=VS.85).aspx : So, why not to implement 'e' letter in open()? it is true crossplatform. On platforms where inheritance does not work, flag should be ignored. P.S. Moreover, I think that all file-descriptor-crete functions (open, socket, pipe, dup, ...) should set CLOEXEC atomically |
FreeBSD 8+ also supports O_CLOEXEC in open(). |
How do you suggest to use it? Even on Windows, python calls open(). And lpSecurityAttributes is an argument of CreateFile (which is the win32 kernel function that open() calls) |
Windows provides a _get_osfhandle() function. There is not the opposite function? :-) Anyway, O_CLOEXEC is not available on all platforms. Even on FreeBSD and Linux, it depends on the OS/kernel version. |
Some times ago, Python has used fopen() for open() implementation. Now, it uses OS-kernel native function to open files. AFAIK, open() in Windows is a wrapper around CreateFile, created to support some POSIX programs in Windows. Why not to use CreateFile() on Windows platform? |
|
See also issue bpo-12760 (Add create mode to open). |
Done, issue bpo-12939. |
Why it is closed as duplicate? nothing said about CLOEXEC in bpo-12797 |
|
And to be explicit, you can now write: def open_cloexex(filename, mode='r'):
return open(filename, mode,
opener=lambda path, mod: os.open(path, mod|os.O_CLOEXEC)) |
Well, I understand. So why not to add 'e' (and 'N', which is the same meaning) character, which:
Also, implement O_CLOEXEC for open() in Windows using appropriate securityattributes with CreateFile() ? |
|
Note that on Windows there is an O_NOINHERIT flag which almost corresponds to O_CLOEXEC on Linux. I don't think there is a need to use the win32 api. |
Ah yes. Because this issue is closed, I created the issue bpo-16850 which is more specific to open + close-and-exec. |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: