Set CLOEXEC for all fds on unix by default #24034
Conversation
|
r? @brson (rust_highfive has picked a reviewer for you, use r? to override) |
| unsafe { libc::_exit(1) } | ||
| } | ||
|
|
||
| rustrt::rust_unset_sigprocmask(); |
There was a problem hiding this comment.
I'll point out that I deleted this and did not re-add it. I personally have no idea why this is here, and I can't really figure it out otherwise. I'd be fine re-adding though!
|
Ack, this r? request fell through my inbox. Reading now... |
|
Looks great! Thanks @alexcrichton. @bors: r+ |
|
📌 Commit 0999ece has been approved by |
|
Oh, and please file a bug for finishing this work by using the atomic APIs whenever available. |
|
⌛ Testing commit 0999ece with merge e787ff3... |
|
⛄ The build was interrupted to prioritize another pull request. |
|
⌛ Testing commit 0999ece with merge cd83aec... |
|
⛄ The build was interrupted to prioritize another pull request. |
|
🔒 Merge conflict |
|
☔ The latest upstream changes (presumably #24232) made this pull request unmergeable. Please resolve the merge conflicts. |
This commit starts to set the CLOEXEC flag for all files and sockets opened by the standard library by default on all unix platforms. There are a few points of note in this commit: * The implementation is not 100% satisfactory in the face of threads. File descriptors only have the `F_CLOEXEC` flag set *after* they are opened, allowing for a fork/exec to happen in the middle and leak the descriptor. Some platforms do support atomically opening a descriptor while setting the `CLOEXEC` flag, and it is left as a future extension to bind these apis as it is unclear how to do so nicely at this time. * The implementation does not offer a method of opting into the old behavior of not setting `CLOEXEC`. This will possibly be added in the future through extensions on `OpenOptions`, for example. * This change does not yet audit any Windows APIs to see if the handles are inherited by default by accident. This is a breaking change for users who call `fork` or `exec` outside of the standard library itself and expect file descriptors to be inherted. All file descriptors created by the standard library will no longer be inherited. [breaking-change]
* De-indent quite a bit by removing usage of FnOnce closures
* Clearly separate code for the parent/child after the fork
* Use `fs2::{File, OpenOptions}` instead of calling `open` manually
* Use RAII to close I/O objects wherever possible
* Remove loop for closing all file descriptors, all our own ones are now
`CLOEXEC` by default so they cannot be inherited
|
@bors: r=aturon |
|
📌 Commit 551e15d has been approved by |
|
⌛ Testing commit 551e15d with merge a73ce4c... |
|
💔 Test failed - auto-linux-64-x-android-t |
The logic for only closing file descriptors >= 3 was inherited from quite some time ago and ends up meaning that some internal APIs are less consistent than they should be. By unconditionally closing everything entering a `FileDesc` we ensure that we're consistent in our behavior as well as robustly handling the stdio case.
|
⌛ Testing commit eadc3bc with merge f367bd1... |
|
⛄ The build was interrupted to prioritize another pull request. |
The commit messages have more details as to what's going on, but this is a breaking change for any libraries which expect file descriptors to be inherited by default. Closes #12148
The commit messages have more details as to what's going on, but this is a breaking change for any libraries which expect file descriptors to be inherited by default.
Closes #12148