-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Finish atomically setting CLOEXEC for fds created on Unix #31417
Conversation
r? @brson (rust_highfive has picked a reviewer for you, use r? to override) |
Ah I should also mention, PEP 433 was quite a valuable resource in terms of a reference to look at. It contains lots of information plus lots of research about the minimum kernel versions needed to support various newer syscalls here and there. |
} | ||
} | ||
|
||
const RTLD_LAZY: libc::c_int = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this is also being added to libc, and I don't mind waiting on merging this until that's added.
9cff172
to
229ce68
Compare
r=me with nits |
On Linux we have to do this for binary compatibility with 2.6.18, but for other OSes (e.g. OSX/BSDs/etc) they all support this flag so we don't need to pass it.
Similar to the previous commit, if `F_DUPFD_CLOEXEC` succeeds then there's no need for us to then call `set_cloexec` on platforms other than Linux. The bug mentioned of kernels not actually setting the `CLOEXEC` flag has only been repored on Linux, not elsewhere.
This commit adds support for creating sockets with the `SOCK_CLOEXEC` flag. Support for this flag was added in Linux 2.6.27, however, and support does not exist on platforms other than Linux. For this reason we still have the same fallback as before but just special case Linux if we can.
Right now we only attempt to call one symbol which my not exist everywhere, __pthread_get_minstack, but this pattern will come up more often as we start to bind newer functionality of systems like Linux. Take a similar strategy as the Windows implementation where we use `dlopen` to lookup whether a symbol exists or not.
This is necessary to atomically accept a socket and set the CLOEXEC flag at the same time. Support only appeared in Linux 2.6.28 so we have to dynamically determine which syscall we're supposed to call in this case.
This commit attempts to use the `pipe2` syscall on Linux to atomically set the CLOEXEC flag for pipes created. Unfortunately this was added in 2.6.27 so we have to dynamically determine whether we can use it or not. This commit also updates the `fds-are-cloexec.rs` test to test stdio handles for spawned processes as well.
229ce68
to
812b309
Compare
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
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 fortry_clone
was added in #27980. This commit fills out:Socket::new
now passesSOCK_CLOEXEC
Socket::accept
now usesaccept4
pipe2
is used instead ofpipe
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