From 1cc9718fdef63476ffdf3f0bcd74b554b083f378 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 1 Jan 2015 10:19:42 -0800 Subject: [PATCH] Revert "Revert "std: Re-enable at_exit()"" This reverts commit aec67c2ee0f673ea7b0e21c2fe7e0f26a523d823. --- src/liblog/lib.rs | 40 +++++++++++++------ src/libstd/rt/mod.rs | 21 +++------- src/libstd/rt/unwind.rs | 23 ++++++++--- src/libstd/sys/common/helper_thread.rs | 19 ++++++--- src/libstd/sys/common/thread_local.rs | 4 -- src/libstd/sys/unix/timer.rs | 9 ++++- src/libstd/sys/windows/thread_local.rs | 12 ++++-- src/libstd/sys/windows/timer.rs | 5 ++- .../run-fail/rt-set-exit-status-panic2.rs | 2 +- src/test/run-pass/unique-send-2.rs | 2 +- 10 files changed, 86 insertions(+), 51 deletions(-) diff --git a/src/liblog/lib.rs b/src/liblog/lib.rs index c634a46888e56..4537fc763c953 100644 --- a/src/liblog/lib.rs +++ b/src/liblog/lib.rs @@ -183,10 +183,9 @@ use std::io::{self, Stderr}; use std::io::prelude::*; use std::mem; use std::env; -use std::ptr; use std::rt; use std::slice; -use std::sync::{Once, ONCE_INIT}; +use std::sync::{Once, ONCE_INIT, StaticMutex, MUTEX_INIT}; use directive::LOG_LEVEL_NAMES; @@ -202,6 +201,8 @@ pub const MAX_LOG_LEVEL: u32 = 255; /// The default logging level of a crate if no other is specified. const DEFAULT_LOG_LEVEL: u32 = 1; +static LOCK: StaticMutex = MUTEX_INIT; + /// An unsafe constant that is the maximum logging level of any module /// specified. This is the first line of defense to determining whether a /// logging statement should be run. @@ -286,9 +287,18 @@ impl Drop for DefaultLogger { pub fn log(level: u32, loc: &'static LogLocation, args: fmt::Arguments) { // Test the literal string from args against the current filter, if there // is one. - match unsafe { FILTER.as_ref() } { - Some(filter) if !args.to_string().contains(&filter[..]) => return, - _ => {} + unsafe { + let _g = LOCK.lock(); + match FILTER as uint { + 0 => {} + 1 => panic!("cannot log after main thread has exited"), + n => { + let filter = mem::transmute::<_, &String>(n); + if !args.to_string().contains(&filter[..]) { + return + } + } + } } // Completely remove the local logger from TLS in case anyone attempts to @@ -370,9 +380,15 @@ pub fn mod_enabled(level: u32, module: &str) -> bool { // This assertion should never get tripped unless we're in an at_exit // handler after logging has been torn down and a logging attempt was made. - assert!(unsafe { !DIRECTIVES.is_null() }); - enabled(level, module, unsafe { (*DIRECTIVES).iter() }) + let _g = LOCK.lock(); + unsafe { + assert!(DIRECTIVES as uint != 0); + assert!(DIRECTIVES as uint != 1, + "cannot log after the main thread has exited"); + + enabled(level, module, (*DIRECTIVES).iter()) + } } fn enabled(level: u32, @@ -428,14 +444,14 @@ fn init() { // Schedule the cleanup for the globals for when the runtime exits. rt::at_exit(move || { + let _g = LOCK.lock(); assert!(!DIRECTIVES.is_null()); - let _directives: Box> = - Box::from_raw(DIRECTIVES); - DIRECTIVES = ptr::null_mut(); + let _directives = Box::from_raw(DIRECTIVES); + DIRECTIVES = 1 as *mut _; if !FILTER.is_null() { - let _filter: Box = Box::from_raw(FILTER); - FILTER = 0 as *mut _; + let _filter = Box::from_raw(FILTER); + FILTER = 1 as *mut _; } }); } diff --git a/src/libstd/rt/mod.rs b/src/libstd/rt/mod.rs index 90cc189b9a0f0..20e29998d7c05 100644 --- a/src/libstd/rt/mod.rs +++ b/src/libstd/rt/mod.rs @@ -147,20 +147,14 @@ fn lang_start(main: *const u8, argc: int, argv: *const *const u8) -> int { } } -/// Enqueues a procedure to run when the runtime is cleaned up -/// -/// The procedure passed to this function will be executed as part of the -/// runtime cleanup phase. For normal rust programs, this means that it will run -/// after all other threads have exited. -/// -/// The procedure is *not* executed with a local `Thread` available to it, so -/// primitives like logging, I/O, channels, spawning, etc, are *not* available. -/// This is meant for "bare bones" usage to clean up runtime details, this is -/// not meant as a general-purpose "let's clean everything up" function. +/// Enqueues a procedure to run when the main thread exits. /// /// It is forbidden for procedures to register more `at_exit` handlers when they /// are running, and doing so will lead to a process abort. -pub fn at_exit(f: F) { +/// +/// Note that other threads may still be running when `at_exit` routines start +/// running. +pub fn at_exit(f: F) { at_exit_imp::push(Thunk::new(f)); } @@ -176,8 +170,5 @@ pub fn at_exit(f: F) { pub unsafe fn cleanup() { args::cleanup(); sys::stack_overflow::cleanup(); - // FIXME: (#20012): the resources being cleaned up by at_exit - // currently are not prepared for cleanup to happen asynchronously - // with detached threads using the resources; for now, we leak. - // at_exit_imp::cleanup(); + at_exit_imp::cleanup(); } diff --git a/src/libstd/rt/unwind.rs b/src/libstd/rt/unwind.rs index ebb2a2e4827a1..3ee3954ed6434 100644 --- a/src/libstd/rt/unwind.rs +++ b/src/libstd/rt/unwind.rs @@ -69,7 +69,7 @@ use intrinsics; use libc::c_void; use mem; use sync::atomic::{self, Ordering}; -use sync::{Once, ONCE_INIT}; +use sys_common::mutex::{Mutex, MUTEX_INIT}; use rt::libunwind as uw; @@ -534,11 +534,22 @@ pub fn begin_unwind(msg: M, file_line: &(&'static str, uint)) -> /// Doing this split took the LLVM IR line counts of `fn main() { panic!() /// }` from ~1900/3700 (-O/no opts) to 180/590. #[inline(never)] #[cold] // this is the slow path, please never inline this -fn begin_unwind_inner(msg: Box, file_line: &(&'static str, uint)) -> ! { - // Make sure the default panic handler is registered before we look at the - // callbacks. - static INIT: Once = ONCE_INIT; - INIT.call_once(|| unsafe { register(panicking::on_panic); }); +fn begin_unwind_inner(msg: Box, + file_line: &(&'static str, uint)) -> ! { + // Make sure the default failure handler is registered before we look at the + // callbacks. We also use a raw sys-based mutex here instead of a + // `std::sync` one as accessing TLS can cause weird recursive problems (and + // we don't need poison checking). + unsafe { + static LOCK: Mutex = MUTEX_INIT; + static mut INIT: bool = false; + LOCK.lock(); + if !INIT { + register(panicking::on_panic); + INIT = true; + } + LOCK.unlock(); + } // First, invoke call the user-defined callbacks triggered on thread panic. // diff --git a/src/libstd/sys/common/helper_thread.rs b/src/libstd/sys/common/helper_thread.rs index 3b5fd5a571460..2a852fbcd57e3 100644 --- a/src/libstd/sys/common/helper_thread.rs +++ b/src/libstd/sys/common/helper_thread.rs @@ -24,7 +24,6 @@ use prelude::v1::*; use boxed; use cell::UnsafeCell; -use ptr; use rt; use sync::{StaticMutex, StaticCondvar}; use sync::mpsc::{channel, Sender, Receiver}; @@ -97,7 +96,7 @@ impl Helper { { unsafe { let _guard = self.lock.lock().unwrap(); - if !*self.initialized.get() { + if *self.chan.get() as uint == 0 { let (tx, rx) = channel(); *self.chan.get() = boxed::into_raw(box tx); let (receive, send) = helper_signal::new(); @@ -113,8 +112,10 @@ impl Helper { self.cond.notify_one() }); - rt::at_exit(move|| { self.shutdown() }); + rt::at_exit(move || { self.shutdown() }); *self.initialized.get() = true; + } else if *self.chan.get() as uint == 1 { + panic!("cannot continue usage after shutdown"); } } } @@ -129,7 +130,9 @@ impl Helper { // Must send and *then* signal to ensure that the child receives the // message. Otherwise it could wake up and go to sleep before we // send the message. - assert!(!self.chan.get().is_null()); + assert!(*self.chan.get() as uint != 0); + assert!(*self.chan.get() as uint != 1, + "cannot continue usage after shutdown"); (**self.chan.get()).send(msg).unwrap(); helper_signal::signal(*self.signal.get() as helper_signal::signal); } @@ -142,9 +145,13 @@ impl Helper { // returns. let mut guard = self.lock.lock().unwrap(); + let ptr = *self.chan.get(); + if ptr as uint == 1 { + panic!("cannot continue usage after shutdown"); + } // Close the channel by destroying it - let chan: Box> = Box::from_raw(*self.chan.get()); - *self.chan.get() = ptr::null_mut(); + let chan = Box::from_raw(*self.chan.get()); + *self.chan.get() = 1 as *mut Sender; drop(chan); helper_signal::signal(*self.signal.get() as helper_signal::signal); diff --git a/src/libstd/sys/common/thread_local.rs b/src/libstd/sys/common/thread_local.rs index ecd047710bb96..5e2a138fa63ba 100644 --- a/src/libstd/sys/common/thread_local.rs +++ b/src/libstd/sys/common/thread_local.rs @@ -61,7 +61,6 @@ use prelude::v1::*; use sync::atomic::{self, AtomicUsize, Ordering}; -use sync::{Mutex, Once, ONCE_INIT}; use sys::thread_local as imp; @@ -142,9 +141,6 @@ pub const INIT_INNER: StaticKeyInner = StaticKeyInner { key: atomic::ATOMIC_USIZE_INIT, }; -static INIT_KEYS: Once = ONCE_INIT; -static mut KEYS: *mut Mutex> = 0 as *mut _; - impl StaticKey { /// Gets the value associated with this TLS key /// diff --git a/src/libstd/sys/unix/timer.rs b/src/libstd/sys/unix/timer.rs index ef0274fdda92d..ef175d68fc43f 100644 --- a/src/libstd/sys/unix/timer.rs +++ b/src/libstd/sys/unix/timer.rs @@ -170,8 +170,15 @@ fn helper(input: libc::c_int, messages: Receiver, _: ()) { 1 => { loop { match messages.try_recv() { + // Once we've been disconnected it means the main thread + // is exiting (at_exit has run). We could still have + // active timers for other threads, so we're just going + // to drop them all on the floor. This is all we can + // really do, however, to prevent resource leakage. The + // remaining timers will likely start panicking quickly + // as they attempt to re-use this thread but are + // disallowed to do so. Err(TryRecvError::Disconnected) => { - assert!(active.len() == 0); break 'outer; } diff --git a/src/libstd/sys/windows/thread_local.rs b/src/libstd/sys/windows/thread_local.rs index 30c483ac52fa2..1359803070af3 100644 --- a/src/libstd/sys/windows/thread_local.rs +++ b/src/libstd/sys/windows/thread_local.rs @@ -138,9 +138,9 @@ unsafe fn init_dtors() { rt::at_exit(move|| { DTOR_LOCK.lock(); let dtors = DTORS; - DTORS = ptr::null_mut(); + DTORS = 1 as *mut _; Box::from_raw(dtors); - assert!(DTORS.is_null()); // can't re-init after destructing + assert!(DTORS as uint == 1); // can't re-init after destructing DTOR_LOCK.unlock(); }); } @@ -148,6 +148,9 @@ unsafe fn init_dtors() { unsafe fn register_dtor(key: Key, dtor: Dtor) { DTOR_LOCK.lock(); init_dtors(); + assert!(DTORS as uint != 0); + assert!(DTORS as uint != 1, + "cannot create new TLS keys after the main thread has exited"); (*DTORS).push((key, dtor)); DTOR_LOCK.unlock(); } @@ -155,6 +158,9 @@ unsafe fn register_dtor(key: Key, dtor: Dtor) { unsafe fn unregister_dtor(key: Key) -> bool { DTOR_LOCK.lock(); init_dtors(); + assert!(DTORS as uint != 0); + assert!(DTORS as uint != 1, + "cannot unregister destructors after the main thread has exited"); let ret = { let dtors = &mut *DTORS; let before = dtors.len(); @@ -241,7 +247,7 @@ unsafe fn run_dtors() { any_run = false; let dtors = { DTOR_LOCK.lock(); - let ret = if DTORS.is_null() { + let ret = if DTORS as usize <= 1 { Vec::new() } else { (*DTORS).iter().map(|s| *s).collect() diff --git a/src/libstd/sys/windows/timer.rs b/src/libstd/sys/windows/timer.rs index 91a7f694181f1..9bcae926eeabf 100644 --- a/src/libstd/sys/windows/timer.rs +++ b/src/libstd/sys/windows/timer.rs @@ -80,9 +80,10 @@ fn helper(input: libc::HANDLE, messages: Receiver, _: ()) { None => {} } } + // See the comment in unix::timer for why we don't have any + // asserts here and why we're likely just leaving timers on + // the floor as we exit. Err(TryRecvError::Disconnected) => { - assert_eq!(objs.len(), 1); - assert_eq!(chans.len(), 0); break 'outer; } Err(..) => break diff --git a/src/test/run-fail/rt-set-exit-status-panic2.rs b/src/test/run-fail/rt-set-exit-status-panic2.rs index 775d38c8b3044..fb1e03c234d76 100644 --- a/src/test/run-fail/rt-set-exit-status-panic2.rs +++ b/src/test/run-fail/rt-set-exit-status-panic2.rs @@ -35,7 +35,7 @@ fn r(x:int) -> r { fn main() { error!("whatever"); - let _t = thread::spawn(move|| { + let _t = thread::scoped(move|| { let _i = r(5); }); panic!(); diff --git a/src/test/run-pass/unique-send-2.rs b/src/test/run-pass/unique-send-2.rs index 654ac9a095cb1..e0785779ab3c2 100644 --- a/src/test/run-pass/unique-send-2.rs +++ b/src/test/run-pass/unique-send-2.rs @@ -25,7 +25,7 @@ pub fn main() { let _t = (0..n).map(|i| { expected += i; let tx = tx.clone(); - thread::spawn(move|| { + thread::scoped(move|| { child(&tx, i) }) }).collect::>();