-
Notifications
You must be signed in to change notification settings - Fork 13k
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
TLS dtor panics abort the process #24479
Comments
Triage: no change in behavior here. |
It would be nice if this was a clean and intentional abort, which would probably be achieved whenever we make The For example, in #63804 (comment) I noticed that the message doesn't have anything to do with the way proc macros and libstd('s panic runtime) interact, which is what I initially assumed. |
An especially bad side effect is that, when this happens in a test, the test doesn’t always fail. struct Foo;
impl Drop for Foo {
fn drop(&mut self) {
panic!()
}
}
thread_local!(static FOO: Foo = Foo);
#[test]
pub fn test() {
FOO.with(|_| {});
} $ cargo test
Finished test [unoptimized + debuginfo] target(s) in 0.00s
Running target/debug/deps/panicking_test-85130fa46b54f758
running 1 test
thread 'test test ... test' panicked at 'explicit panic', src/main.rs:5:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
fatal runtime error: failed to initiate panic, error 5
$ echo $?
0 The exit status of Edit: This turns out to be an independent problem, due to libtest failing to actually wait for its test threads to exit; I opened a PR as #81367. |
libtest: Wait for test threads to exit after they report completion Otherwise we can miss bugs where a test reports that it succeeded but then panics within a TLS destructor. Example: ```rust use std::thread::sleep; use std::time::Duration; struct Foo; impl Drop for Foo { fn drop(&mut self) { sleep(Duration::from_secs(1)); panic!() } } thread_local!(static FOO: Foo = Foo); #[test] pub fn test() { FOO.with(|_| {}); } ``` Before this fix, `cargo test` incorrectly reports success. ```console $ cargo test Finished test [unoptimized + debuginfo] target(s) in 0.01s Running target/debug/deps/panicking_test-85130fa46b54f758 running 1 test test test ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s $ echo $? 0 ``` After this fix, the failure is visible. (The entire process is aborted due to rust-lang#24479.) ```console $ cargo test Finished test [unoptimized + debuginfo] target(s) in 0.01s Running target/debug/deps/panicking_test-76180625bc2ee3c9 running 1 test thread 'test' panicked at 'explicit panic', src/main.rs:9:9 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace fatal runtime error: failed to initiate panic, error 5 error: test failed, to rerun pass '--bin panicking-test' Caused by: process didn't exit successfully: `/tmp/panicking-test/target/debug/deps/panicking_test-76180625bc2ee3c9 --nocapture` (signal: 6, SIGABRT: process abort signal) $ echo $? 101 ```
Today a destructor in TLS which panics will abort the process:
When compiled and run (note that it must be run on a recent system due to #19776):
The reason behind this is that the context in which TLS destructors are running is different than the execution of the main thread itself (e.g. there is no try/catch block).
Is this (a) desirable and (b) should we change it?
The text was updated successfully, but these errors were encountered: