exit: explain our expectations for the exit handlers registered in a Rust program #129581
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
RalfJung
commented
Aug 25, 2024
| /// threads, it is required that the exit handler performs suitable synchronization with those | ||
| /// threads. (The alternative to this requirement would be to not run exit handlers at all, which is | ||
| /// considered undesirable. Note that returning from `main` also calls `exit`, so making `exit` an | ||
| /// unsafe operation is not an option.) |
There was a problem hiding this comment.
@comex sketched an approach a Rust library could take when wrapping a C library with a problematic destructor, to still uphold this requirement. I don't know what the conclusion was regarding whether this would actually work, and I am not sure if it's worth spelling that out in these docs -- this seems more like something to go into some book on Rust FFI advice.
RalfJung
added
T-libs-api
|
We discussed this in today's @rust-lang/libs-api meeting. We felt that since this is documenting current reality, rather than adding a new guarantee, this doesn't need an FCP, just a lack of objections. Based on the consensus from the meeting: @bors r+ |
bors
added
S-waiting-on-bors
|
Also, separately, we may want an additional mention (in a separate doc PR) that on some targets simultaneous exits (if any of the exits are not from Rust) may cause UB. |
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Aug 27, 2024
exit: explain our expectations for the exit handlers registered in a Rust program This documents the position of `@Amanieu` and others in rust-lang#126600: a library with an atexit handler that destroys state that other threads could still be working on is buggy. We do not consider it acceptable for a library to say "you must call the following cleanup function before exiting from `main` or calling `exit`". I don't know if this is established `@rust-lang/libs-api` consensus so I presume this will have to go through FCP. Given that Rust supports concurrency, I don't think there is any way to write a sound Rust wrapper around a library that has such a required cleanup function: even if we made `exit` unsafe, and the Rust wrapper used the scope-with-callback approach to ensure it can run cleanup code before returning from the wrapper (like `thread::scope`), one could still call this wrapper in a second thread and then return from `main` while the wrapper runs. Making this sound would require `std` to provide a way to "block" returning from `main`, so that while the wrapper runs returning from `main` waits until the wrapper is done... that just doesn't seem feasible. The `exit` docs do not seem like the best place to document this, but I also couldn't think of a better one.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 27, 2024
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#129401 (Partially stabilize `feature(new_uninit)`) - rust-lang#129581 (exit: explain our expectations for the exit handlers registered in a Rust program) - rust-lang#129634 (Fix tidy to allow `edition = "2024"` in `Cargo.toml`) - rust-lang#129635 (Use unsafe extern blocks throughout the compiler) - rust-lang#129645 (Fix typos in floating-point primitive type docs) - rust-lang#129648 (More `unreachable_pub`) - rust-lang#129649 (ABI compat check: detect unadjusted ABI mismatches) - rust-lang#129652 (fix Pointer to reference conversion docs) r? `@ghost` `@rustbot` modify labels: rollup
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 27, 2024
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#129507 (make it possible to enable const_precise_live_drops per-function) - rust-lang#129581 (exit: explain our expectations for the exit handlers registered in a Rust program) - rust-lang#129634 (Fix tidy to allow `edition = "2024"` in `Cargo.toml`) - rust-lang#129635 (Use unsafe extern blocks throughout the compiler) - rust-lang#129645 (Fix typos in floating-point primitive type docs) - rust-lang#129648 (More `unreachable_pub`) - rust-lang#129649 (ABI compat check: detect unadjusted ABI mismatches) - rust-lang#129652 (fix Pointer to reference conversion docs) r? `@ghost` `@rustbot` modify labels: rollup
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 27, 2024
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#129507 (make it possible to enable const_precise_live_drops per-function) - rust-lang#129581 (exit: explain our expectations for the exit handlers registered in a Rust program) - rust-lang#129634 (Fix tidy to allow `edition = "2024"` in `Cargo.toml`) - rust-lang#129635 (Use unsafe extern blocks throughout the compiler) - rust-lang#129645 (Fix typos in floating-point primitive type docs) - rust-lang#129648 (More `unreachable_pub`) - rust-lang#129649 (ABI compat check: detect unadjusted ABI mismatches) - rust-lang#129652 (fix Pointer to reference conversion docs) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 27, 2024
Rollup merge of rust-lang#129581 - RalfJung:exit, r=joshtriplett exit: explain our expectations for the exit handlers registered in a Rust program This documents the position of ``@Amanieu`` and others in rust-lang#126600: a library with an atexit handler that destroys state that other threads could still be working on is buggy. We do not consider it acceptable for a library to say "you must call the following cleanup function before exiting from `main` or calling `exit`". I don't know if this is established ``@rust-lang/libs-api`` consensus so I presume this will have to go through FCP. Given that Rust supports concurrency, I don't think there is any way to write a sound Rust wrapper around a library that has such a required cleanup function: even if we made `exit` unsafe, and the Rust wrapper used the scope-with-callback approach to ensure it can run cleanup code before returning from the wrapper (like `thread::scope`), one could still call this wrapper in a second thread and then return from `main` while the wrapper runs. Making this sound would require `std` to provide a way to "block" returning from `main`, so that while the wrapper runs returning from `main` waits until the wrapper is done... that just doesn't seem feasible. The `exit` docs do not seem like the best place to document this, but I also couldn't think of a better one.
dtolnay
removed
the
I-libs-api-nominated
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This documents the position of @Amanieu and others in #126600: a library with an atexit handler that destroys state that other threads could still be working on is buggy. We do not consider it acceptable for a library to say "you must call the following cleanup function before exiting from
mainor callingexit". I don't know if this is established @rust-lang/libs-api consensus so I presume this will have to go through FCP.Given that Rust supports concurrency, I don't think there is any way to write a sound Rust wrapper around a library that has such a required cleanup function: even if we made
exitunsafe, and the Rust wrapper used the scope-with-callback approach to ensure it can run cleanup code before returning from the wrapper (likethread::scope), one could still call this wrapper in a second thread and then return frommainwhile the wrapper runs. Making this sound would requirestdto provide a way to "block" returning frommain, so that while the wrapper runs returning frommainwaits until the wrapper is done... that just doesn't seem feasible.The
exitdocs do not seem like the best place to document this, but I also couldn't think of a better one.