Skip to content

Commit

Permalink
std: Don't assume thread::current() works on panic
Browse files Browse the repository at this point in the history
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
  • Loading branch information
alexcrichton committed Apr 27, 2015
1 parent 0e154aa commit d98ab4f
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 11 deletions.
6 changes: 3 additions & 3 deletions src/libstd/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use any::Any;
use cell::RefCell;
use rt::{backtrace, unwind};
use sys::stdio::Stderr;
use thread;
use sys_common::thread_info;

thread_local! {
pub static LOCAL_STDERR: RefCell<Option<Box<Write + Send>>> = {
Expand All @@ -34,8 +34,8 @@ pub fn on_panic(obj: &(Any+Send), file: &'static str, line: u32) {
}
};
let mut err = Stderr::new();
let thread = thread::current();
let name = thread.name().unwrap_or("<unnamed>");
let thread = thread_info::current_thread();
let name = thread.as_ref().and_then(|t| t.name()).unwrap_or("<unnamed>");
let prev = LOCAL_STDERR.with(|s| s.borrow_mut().take());
match prev {
Some(mut stderr) => {
Expand Down
11 changes: 5 additions & 6 deletions src/libstd/sys/common/thread_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,9 @@ struct ThreadInfo {
thread_local! { static THREAD_INFO: RefCell<Option<ThreadInfo>> = RefCell::new(None) }

impl ThreadInfo {
fn with<R, F>(f: F) -> R where F: FnOnce(&mut ThreadInfo) -> R {
fn with<R, F>(f: F) -> Option<R> where F: FnOnce(&mut ThreadInfo) -> R {
if THREAD_INFO.state() == LocalKeyState::Destroyed {
panic!("Use of std::thread::current() is not possible after \
the thread's local data has been destroyed");
return None
}

THREAD_INFO.with(move |c| {
Expand All @@ -38,16 +37,16 @@ impl ThreadInfo {
thread: NewThread::new(None),
})
}
f(c.borrow_mut().as_mut().unwrap())
Some(f(c.borrow_mut().as_mut().unwrap()))
})
}
}

pub fn current_thread() -> Thread {
pub fn current_thread() -> Option<Thread> {
ThreadInfo::with(|info| info.thread.clone())
}

pub fn stack_guard() -> usize {
pub fn stack_guard() -> Option<usize> {
ThreadInfo::with(|info| info.stack_guard)
}

Expand Down
2 changes: 1 addition & 1 deletion src/libstd/sys/unix/stack_overflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ mod imp {
// We're calling into functions with stack checks
stack::record_sp_limit(0);

let guard = thread_info::stack_guard();
let guard = thread_info::stack_guard().unwrap_or(0);
let addr = (*info).si_addr as usize;

if guard == 0 || addr < guard - PAGE_SIZE || addr >= guard {
Expand Down
4 changes: 3 additions & 1 deletion src/libstd/thread/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,9 @@ pub fn scoped<'a, T, F>(f: F) -> JoinGuard<'a, T> where
/// Gets a handle to the thread that invokes it.
#[stable(feature = "rust1", since = "1.0.0")]
pub fn current() -> Thread {
thread_info::current_thread()
thread_info::current_thread().expect("use of std::thread::current() is not \
possible after the thread's local \
data has been destroyed")
}

/// Cooperatively gives up a timeslice to the OS scheduler.
Expand Down
39 changes: 39 additions & 0 deletions src/test/run-pass/issue-24313.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use std::thread;
use std::env;
use std::process::Command;

struct Handle(i32);

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

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

fn main() {
let args = env::args().collect::<Vec<_>>();
if args.len() == 1 {
let out = Command::new(&args[0]).arg("test").output().unwrap();
let stderr = std::str::from_utf8(&out.stderr).unwrap();
assert!(stderr.contains("panicked at 'explicit panic'"),
"bad failure message:\n{}\n", stderr);
} else {
// TLS dtors are not always run on process exit
thread::spawn(|| {
HANDLE.with(|h| {
println!("{}", h.0);
});
}).join().unwrap();
}
}

0 comments on commit d98ab4f

Please sign in to comment.