Skip to content

Commit

Permalink
Rollup merge of rust-lang#50638 - tbu-:pr_open_cloexec_once, r=nagisa
Browse files Browse the repository at this point in the history
Don't unconditionally set CLOEXEC twice on every fd we open on Linux

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).
  • Loading branch information
kennytm authored May 16, 2018
2 parents 2a3f536 + 6d1da82 commit d623f45
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 4 deletions.
7 changes: 7 additions & 0 deletions src/libstd/sys/unix/fd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,13 @@ impl FileDesc {
}
}

#[cfg(target_os = "linux")]
pub fn get_cloexec(&self) -> io::Result<bool> {
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",
Expand Down
41 changes: 37 additions & 4 deletions src/libstd/sys/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}

Expand Down

0 comments on commit d623f45

Please sign in to comment.