-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Leverage atomically setting CLOEXEC where possible #24237
Comments
cc #24307 |
tbu-
added a commit
to tbu-/rust
that referenced
this issue
Aug 24, 2015
On Linux the flag is just ignored if it is not supported: https://lwn.net/Articles/588444/ Touches rust-lang#24237.
bors
added a commit
that referenced
this issue
Aug 25, 2015
On Linux the flag is just ignored if it is not supported: https://lwn.net/Articles/588444/ Still needs the values of O_CLOEXEC on the BSDs. Touches #24237.
bors
added a commit
that referenced
this issue
Aug 31, 2015
Still needs values of F_DUPFD_CLOEXEC on other OSs. For Bitrig, NetBSD and OpenBSD the constant was incorrectly in posix01, when it's actually posix08. In order to maintain backwards-compatiblity, the constant was only copied, not moved. cc #24237
bors
added a commit
that referenced
this issue
Feb 6, 2016
These commits finish up closing out #24237 by filling out all locations we create new file descriptors with variants that atomically create the file descriptor and set CLOEXEC where possible. Previous support for doing this in `File::open` was added in #27971 and support for `try_clone` was added in #27980. This commit fills out: * `Socket::new` now passes `SOCK_CLOEXEC` * `Socket::accept` now uses `accept4` * `pipe2` is used instead of `pipe` Unfortunately most of this support is Linux-specific, and most of it is post-2.6.18 (our oldest supported version), so all of the detection here is done dynamically. It looks like OSX does not have equivalent variants for these functions, so there's nothing more we can do there. Support for BSDs can be added over time if they also have these functions. Closes #24237
I know this is closed, but I just have to comment on how well thought out and organised the original description was - how clearly the problem is captured and then it's solution tracked. Well done everyone. |
3 tasks
krallin
added a commit
to krallin/perf-event
that referenced
this issue
Mar 10, 2023
Note: this is a behavior change. Currently, this crate opens FDs without CLOEXEC, which means they persist across exec. This is not the convention throughout the Rust ecosystem, which is to do the opposite: rust-lang/rust#24237 This patch setting CLOEXEC :) Note that doing that after-the-fact on the resulting FD isn't quite good enough: if your process is forking a lot, that's racy.
Phantomical
added a commit
to Phantomical/perf-event
that referenced
this issue
Apr 14, 2023
This appears to be what the rust stdlib does (see [0]) so following along with their conventions is probably best. This commit was inspired by this pr to perf-event: jimblandy/perf-event#29 [0]: rust-lang/rust#24237
Phantomical
added a commit
to Phantomical/perf-event
that referenced
this issue
Apr 20, 2023
This appears to be what the rust stdlib does (see [0]) so following along with their conventions is probably best. This commit was inspired by this pr to perf-event: jimblandy/perf-event#29 [0]: rust-lang/rust#24237
Phantomical
added a commit
to Phantomical/perf-event
that referenced
this issue
Apr 20, 2023
This appears to be what the rust stdlib does (see [0]) so following along with their conventions is probably best. This commit was inspired by this pr to perf-event: jimblandy/perf-event#29 [0]: rust-lang/rust#24237
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
After #24034 lands we will be setting
CLOEXEC
on all file descriptors on unix in the standard library. Currently we do so in a nonatomic fashion, however, so it's possible to continue to leak file descriptors into children if a thread is concurrently forking.Many platforms have methods of creating
CLOEXEC
file descriptors atomically, but many of the APIs are quite new and they need to be properly detected at runtime. For example, the current instances offd.set_cloexec()
that need to be migrated are listed below. A reminder of our minimum platform requirements are linux 2.6.18 and OSX 10.7.File::open
- this should specify theO_CLOEXEC
flag. This flag was added in linux 2.6.23 and appears to exist on OSX 10.7. Atomically open files with O_CLOEXEC where possible #27971Socket::new
- this should be specify theSOCK_CLOEXEC
flag in thetype
argument (second). This flag was only added in linux 2.6.27 and does not exist on OSX. Finish atomically setting CLOEXEC for fds created on Unix #31417Socket::accept
- this should be replaced with a call toaccept4
. This system call was only added in linux 2.6.28 and does not exist on OSX. Finish atomically setting CLOEXEC for fds created on Unix #31417Socket::duplicate
- this should be replaced withdup3
(linux 2.6.27, not on OSX) orF_DUPFD_CLOEXEC
(linux 2.6.24, appears to exist on OSX 10.7). Atomically set CLOEXEC on duplicated sockets #27980anon_pipe
- this could be replaced withpipe2
on linux 2.6.27 and perhapssocketpair
on OSX (not confirmed). Finish atomically setting CLOEXEC for fds created on Unix #31417The hard part about this bug is figuring out if it's possible to avoid compile-time detection of these functions (and instead rely on runtime detection).
The text was updated successfully, but these errors were encountered: