From 6d1da8232997af4b785486329f01995440818920 Mon Sep 17 00:00:00 2001 From: Tobias Bucher Date: Mon, 14 May 2018 13:20:39 +0200 Subject: [PATCH] Don't unconditionally set CLOEXEC twice on every fd we open on Linux MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, every `open64` was accompanied by a `ioctl(…, FIOCLEX)`, because some old Linux version would ignore the `O_CLOEXEC` flag we pass to the `open64` function. Now, we check whether the `CLOEXEC` flag is set on the first file we open – if it is, we won't do extra syscalls for every opened file. If it is not set, we fall back to the old behavior of unconditionally calling `ioctl(…, FIOCLEX)` on newly opened files. On old Linuxes, this amounts to one extra syscall per process, namely the `fcntl(…, F_GETFD)` call to check the `CLOEXEC` flag. On new Linuxes, this reduces the number of syscalls per opened file by one, except for the first file, where it does the same number of syscalls as before (`fcntl(…, F_GETFD)` to check the flag instead of `ioctl(…, FIOCLEX)` to set it). --- src/libstd/sys/unix/fd.rs | 7 +++++++ src/libstd/sys/unix/fs.rs | 41 +++++++++++++++++++++++++++++++++++---- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/src/libstd/sys/unix/fd.rs b/src/libstd/sys/unix/fd.rs index 5dafc3251e755..72272d0581a79 100644 --- a/src/libstd/sys/unix/fd.rs +++ b/src/libstd/sys/unix/fd.rs @@ -140,6 +140,13 @@ impl FileDesc { } } + #[cfg(target_os = "linux")] + pub fn get_cloexec(&self) -> io::Result { + unsafe { + Ok((cvt(libc::fcntl(self.fd, libc::F_GETFD))? & libc::FD_CLOEXEC) != 0) + } + } + #[cfg(not(any(target_env = "newlib", target_os = "solaris", target_os = "emscripten", diff --git a/src/libstd/sys/unix/fs.rs b/src/libstd/sys/unix/fs.rs index a1ca839dc1872..77968ffdedf37 100644 --- a/src/libstd/sys/unix/fs.rs +++ b/src/libstd/sys/unix/fs.rs @@ -441,15 +441,48 @@ impl File { // Currently the standard library supports Linux 2.6.18 which did not // have the O_CLOEXEC flag (passed above). If we're running on an older - // Linux kernel then the flag is just ignored by the OS, so we continue - // to explicitly ask for a CLOEXEC fd here. + // Linux kernel then the flag is just ignored by the OS. After we open + // the first file, we check whether it has CLOEXEC set. If it doesn't, + // we will explicitly ask for a CLOEXEC fd for every further file we + // open, if it does, we will skip that step. // // The CLOEXEC flag, however, is supported on versions of macOS/BSD/etc // that we support, so we only do this on Linux currently. - if cfg!(target_os = "linux") { - fd.set_cloexec()?; + #[cfg(target_os = "linux")] + fn ensure_cloexec(fd: &FileDesc) -> io::Result<()> { + use sync::atomic::{AtomicUsize, Ordering}; + + const OPEN_CLOEXEC_UNKNOWN: usize = 0; + const OPEN_CLOEXEC_SUPPORTED: usize = 1; + const OPEN_CLOEXEC_NOTSUPPORTED: usize = 2; + static OPEN_CLOEXEC: AtomicUsize = AtomicUsize::new(OPEN_CLOEXEC_UNKNOWN); + + let need_to_set; + match OPEN_CLOEXEC.load(Ordering::Relaxed) { + OPEN_CLOEXEC_UNKNOWN => { + need_to_set = !fd.get_cloexec()?; + OPEN_CLOEXEC.store(if need_to_set { + OPEN_CLOEXEC_NOTSUPPORTED + } else { + OPEN_CLOEXEC_SUPPORTED + }, Ordering::Relaxed); + }, + OPEN_CLOEXEC_SUPPORTED => need_to_set = false, + OPEN_CLOEXEC_NOTSUPPORTED => need_to_set = true, + _ => unreachable!(), + } + if need_to_set { + fd.set_cloexec()?; + } + Ok(()) + } + + #[cfg(not(target_os = "linux"))] + fn ensure_cloexec(_: &FileDesc) -> io::Result<()> { + Ok(()) } + ensure_cloexec(&fd)?; Ok(File(fd)) }