Skip to content

Commit

Permalink
Auto merge of #24478 - alexcrichton:issue-24313, r=aturon
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
bors committed Apr 28, 2015
2 parents 97d4e76 + d98ab4f commit 2b8c9b1
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 31 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
24 changes: 4 additions & 20 deletions src/libstd/thread/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ use cell::UnsafeCell;

// Sure wish we had macro hygiene, no?
#[doc(hidden)]
#[unstable(feature = "thread_local_internals")]
pub mod __impl {
pub use super::imp::Key as KeyInner;
pub use super::imp::destroy_value;
Expand Down Expand Up @@ -78,12 +77,10 @@ pub struct LocalKey<T> {
// This is trivially devirtualizable by LLVM because we never store anything
// to this field and rustc can declare the `static` as constant as well.
#[doc(hidden)]
#[unstable(feature = "thread_local_internals")]
pub inner: fn() -> &'static __impl::KeyInner<UnsafeCell<Option<T>>>,

// initialization routine to invoke to create a value
#[doc(hidden)]
#[unstable(feature = "thread_local_internals")]
pub init: fn() -> T,
}

Expand Down Expand Up @@ -297,36 +294,31 @@ impl<T: 'static> LocalKey<T> {
}

#[cfg(all(any(target_os = "macos", target_os = "linux"), not(target_arch = "aarch64")))]
#[doc(hidden)]
mod imp {
use prelude::v1::*;

use cell::UnsafeCell;
use intrinsics;
use ptr;

#[doc(hidden)]
#[unstable(feature = "thread_local_internals")]
pub struct Key<T> {
// Place the inner bits in an `UnsafeCell` to currently get around the
// "only Sync statics" restriction. This allows any type to be placed in
// the cell.
//
// Note that all access requires `T: 'static` so it can't be a type with
// any borrowed pointers still.
#[unstable(feature = "thread_local_internals")]
pub inner: UnsafeCell<T>,

// Metadata to keep track of the state of the destructor. Remember that
// these variables are thread-local, not global.
#[unstable(feature = "thread_local_internals")]
pub dtor_registered: UnsafeCell<bool>, // should be Cell
#[unstable(feature = "thread_local_internals")]
pub dtor_running: UnsafeCell<bool>, // should be Cell
}

unsafe impl<T> ::marker::Sync for Key<T> { }

#[doc(hidden)]
impl<T> Key<T> {
pub unsafe fn get(&'static self) -> Option<&'static T> {
if intrinsics::needs_drop::<T>() && *self.dtor_running.get() {
Expand Down Expand Up @@ -422,8 +414,6 @@ mod imp {
_tlv_atexit(dtor, t);
}

#[doc(hidden)]
#[unstable(feature = "thread_local_internals")]
pub unsafe extern fn destroy_value<T>(ptr: *mut u8) {
let ptr = ptr as *mut Key<T>;
// Right before we run the user destructor be sure to flag the
Expand All @@ -435,6 +425,7 @@ mod imp {
}

#[cfg(any(not(any(target_os = "macos", target_os = "linux")), target_arch = "aarch64"))]
#[doc(hidden)]
mod imp {
use prelude::v1::*;

Expand All @@ -444,16 +435,12 @@ mod imp {
use ptr;
use sys_common::thread_local::StaticKey as OsStaticKey;

#[doc(hidden)]
#[unstable(feature = "thread_local_internals")]
pub struct Key<T> {
// Statically allocated initialization expression, using an `UnsafeCell`
// for the same reasons as above.
#[unstable(feature = "thread_local_internals")]
pub inner: UnsafeCell<T>,

// OS-TLS key that we'll use to key off.
#[unstable(feature = "thread_local_internals")]
pub os: OsStaticKey,
}

Expand All @@ -464,7 +451,6 @@ mod imp {
value: T,
}

#[doc(hidden)]
impl<T> Key<T> {
pub unsafe fn get(&'static self) -> Option<&'static T> {
self.ptr().map(|p| &*p)
Expand All @@ -489,14 +475,12 @@ mod imp {
key: self,
value: mem::transmute_copy(&self.inner),
};
let ptr: *mut Value<T> = boxed::into_raw(ptr);
let ptr = boxed::into_raw(ptr);
self.os.set(ptr as *mut u8);
Some(&mut (*ptr).value as *mut T)
}
}

#[doc(hidden)]
#[unstable(feature = "thread_local_internals")]
pub unsafe extern fn destroy_value<T: 'static>(ptr: *mut u8) {
// The OS TLS ensures that this key contains a NULL value when this
// destructor starts to run. We set it back to a sentinel value of 1 to
Expand All @@ -505,7 +489,7 @@ mod imp {
//
// Note that to prevent an infinite loop we reset it back to null right
// before we return from the destructor ourselves.
let ptr: Box<Value<T>> = Box::from_raw(ptr as *mut Value<T>);
let ptr = Box::from_raw(ptr as *mut Value<T>);
let key = ptr.key;
key.os.set(1 as *mut u8);
drop(ptr);
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 2b8c9b1

Please sign in to comment.