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

std: Don't assume thread::current() works on panic #24478

Merged
merged 2 commits into from
Apr 28, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were these accidentally stripped?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The module at the top-level has an #[unstable] attribute which everything inside inherits from by default. (it was intentional)

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();
}
}