Skip to content
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

Panicking in the drop function of a struct that is stored in a thread local causes a stack overflow #24313

Closed
carllerche opened this issue Apr 11, 2015 · 1 comment · Fixed by #24478
Labels
I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics.

Comments

@carllerche
Copy link
Member

When thread locals are dropped, the thread is not valid anymore so accessing thread::current() will panic, however, begin_unwind calls thread::current() which will cause an infinite loop and a stack overflow.

The follow example has been tested to crash with an out of memory error on OS X:

use std::thread;

struct Handle {
    i: i32,
}

impl Drop for Handle {
    fn drop(&mut self) {
        panic!("BOOM");
    }
}

thread_local!(static HANDLE: Handle = new_handle());

fn new_handle() -> Handle {
    Handle { i: 0 }
}

pub fn main() {
    let _ = thread::scoped(|| {
        HANDLE.with(|h| {
            println!("HANDLE: {}", h.i);
        });
    });
}

cc @alexcrichton

@sfackler sfackler added the I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. label Apr 11, 2015
@abonander
Copy link
Contributor

What would be the preferred fix here? Have thread::current() return some sane value instead of panicking itself?

alexcrichton added a commit to alexcrichton/rust that referenced this issue Apr 15, 2015
Inspecting the current thread's info may not always work due to the TLS value
having been destroyed (or is actively being destroyed). The code for printing
a panic message assumed, however, that it could acquire the thread's name
through this method.

Instead this commit propagates the `Option` outwards to allow the
`std::panicking` module to handle the case where the current thread isn't
present.

While it solves the immediate issue of rust-lang#24313, there is still another underlying
issue of panicking destructors in thread locals will abort the process.

Closes rust-lang#24313
alexcrichton added a commit to alexcrichton/rust that referenced this issue Apr 27, 2015
Inspecting the current thread's info may not always work due to the TLS value
having been destroyed (or is actively being destroyed). The code for printing
a panic message assumed, however, that it could acquire the thread's name
through this method.

Instead this commit propagates the `Option` outwards to allow the
`std::panicking` module to handle the case where the current thread isn't
present.

While it solves the immediate issue of rust-lang#24313, there is still another underlying
issue of panicking destructors in thread locals will abort the process.

Closes rust-lang#24313
alexcrichton added a commit to alexcrichton/rust that referenced this issue Apr 27, 2015
Inspecting the current thread's info may not always work due to the TLS value
having been destroyed (or is actively being destroyed). The code for printing
a panic message assumed, however, that it could acquire the thread's name
through this method.

Instead this commit propagates the `Option` outwards to allow the
`std::panicking` module to handle the case where the current thread isn't
present.

While it solves the immediate issue of rust-lang#24313, there is still another underlying
issue of panicking destructors in thread locals will abort the process.

Closes rust-lang#24313
bors added a commit that referenced this issue Apr 28, 2015
Inspecting the current thread's info may not always work due to the TLS value
having been destroyed (or is actively being destroyed). The code for printing
a panic message assumed, however, that it could acquire the thread's name
through this method.

Instead this commit propagates the `Option` outwards to allow the
`std::panicking` module to handle the case where the current thread isn't
present.

While it solves the immediate issue of #24313, there is still another underlying
issue of panicking destructors in thread locals will abort the process.

Closes #24313
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants