Skip to content

Commit 1ec9adc

Browse files
committed
std: Tweak rt::at_exit behavior
There have been some recent panics on the bots and this commit is an attempt to appease them. Previously it was considered invalid to run `rt::at_exit` after the handlers had already started running. Due to the multithreaded nature of applications, however, it is not always possible to guarantee this. For example [this program][ex] will show off the abort. [ex]: https://gist.github.com/alexcrichton/56300b87af6fa554e52d The semantics of the `rt::at_exit` function have been modified as such: * It is now legal to call `rt::at_exit` at any time. The return value now indicates whether the closure was successfully registered or not. Callers must now decide what to do with this information. * The `rt::at_exit` handlers will now be run for a fixed number of iterations. Common cases (such as the example shown) may end up registering a new handler while others are running perhaps once or twice, so this common condition is covered by re-running the handlers a fixed number of times, after which new registrations are forbidden. Some usage of `rt::at_exit` was updated to handle these new semantics, but deprecated or unstable libraries calling `rt::at_exit` were not updated.
1 parent ecf8c64 commit 1ec9adc

File tree

6 files changed

+65
-46
lines changed

6 files changed

+65
-46
lines changed

src/liblog/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -443,7 +443,7 @@ fn init() {
443443
DIRECTIVES = boxed::into_raw(box directives);
444444

445445
// Schedule the cleanup for the globals for when the runtime exits.
446-
rt::at_exit(move || {
446+
let _ = rt::at_exit(move || {
447447
let _g = LOCK.lock();
448448
assert!(!DIRECTIVES.is_null());
449449
let _directives = Box::from_raw(DIRECTIVES);

src/libstd/io/lazy.rs

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,25 +35,33 @@ impl<T: Send + Sync + 'static> Lazy<T> {
3535
pub fn get(&'static self) -> Option<Arc<T>> {
3636
let _g = self.lock.lock();
3737
unsafe {
38-
let mut ptr = *self.ptr.get();
38+
let ptr = *self.ptr.get();
3939
if ptr.is_null() {
40-
ptr = boxed::into_raw(self.init());
41-
*self.ptr.get() = ptr;
40+
Some(self.init())
4241
} else if ptr as usize == 1 {
43-
return None
42+
None
43+
} else {
44+
Some((*ptr).clone())
4445
}
45-
Some((*ptr).clone())
4646
}
4747
}
4848

49-
fn init(&'static self) -> Box<Arc<T>> {
50-
rt::at_exit(move || unsafe {
49+
unsafe fn init(&'static self) -> Arc<T> {
50+
// If we successfully register an at exit handler, then we cache the
51+
// `Arc` allocation in our own internal box (it will get deallocated by
52+
// the at exit handler). Otherwise we just return the freshly allocated
53+
// `Arc`.
54+
let registered = rt::at_exit(move || {
5155
let g = self.lock.lock();
5256
let ptr = *self.ptr.get();
5357
*self.ptr.get() = 1 as *mut _;
5458
drop(g);
5559
drop(Box::from_raw(ptr))
5660
});
57-
Box::new((self.init)())
61+
let ret = (self.init)();
62+
if registered.is_ok() {
63+
*self.ptr.get() = boxed::into_raw(Box::new(ret.clone()));
64+
}
65+
return ret
5866
}
5967
}

src/libstd/old_io/stdio.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ pub fn stdin() -> StdinReader {
238238
STDIN = boxed::into_raw(box stdin);
239239

240240
// Make sure to free it at exit
241-
rt::at_exit(|| {
241+
let _ = rt::at_exit(|| {
242242
Box::from_raw(STDIN);
243243
STDIN = ptr::null_mut();
244244
});

src/libstd/rt/at_exit_imp.rs

Lines changed: 36 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@
1212
//!
1313
//! Documentation can be found on the `rt::at_exit` function.
1414
15+
// FIXME: switch this to use atexit. Currently this
16+
// segfaults (the queue's memory is mysteriously gone), so
17+
// instead the cleanup is tied to the `std::rt` entry point.
18+
1519
use boxed;
1620
use boxed::Box;
1721
use vec::Vec;
@@ -27,47 +31,56 @@ type Queue = Vec<Thunk<'static>>;
2731
static LOCK: Mutex = MUTEX_INIT;
2832
static mut QUEUE: *mut Queue = 0 as *mut Queue;
2933

30-
unsafe fn init() {
34+
// The maximum number of times the cleanup routines will be run. While running
35+
// the at_exit closures new ones may be registered, and this count is the number
36+
// of times the new closures will be allowed to register successfully. After
37+
// this number of iterations all new registrations will return `false`.
38+
const ITERS: usize = 10;
39+
40+
unsafe fn init() -> bool {
3141
if QUEUE.is_null() {
3242
let state: Box<Queue> = box Vec::new();
3343
QUEUE = boxed::into_raw(state);
34-
} else {
44+
} else if QUEUE as usize == 1 {
3545
// can't re-init after a cleanup
36-
rtassert!(QUEUE as uint != 1);
46+
return false
3747
}
3848

39-
// FIXME: switch this to use atexit as below. Currently this
40-
// segfaults (the queue's memory is mysteriously gone), so
41-
// instead the cleanup is tied to the `std::rt` entry point.
42-
//
43-
// ::libc::atexit(cleanup);
49+
return true
4450
}
4551

4652
pub fn cleanup() {
47-
unsafe {
48-
LOCK.lock();
49-
let queue = QUEUE;
50-
QUEUE = 1 as *mut _;
51-
LOCK.unlock();
53+
for i in 0..ITERS {
54+
unsafe {
55+
LOCK.lock();
56+
let queue = QUEUE;
57+
QUEUE = if i == ITERS - 1 {1} else {0} as *mut _;
58+
LOCK.unlock();
5259

53-
// make sure we're not recursively cleaning up
54-
rtassert!(queue as uint != 1);
60+
// make sure we're not recursively cleaning up
61+
rtassert!(queue as usize != 1);
5562

56-
// If we never called init, not need to cleanup!
57-
if queue as uint != 0 {
58-
let queue: Box<Queue> = Box::from_raw(queue);
59-
for to_run in *queue {
60-
to_run.invoke(());
63+
// If we never called init, not need to cleanup!
64+
if queue as usize != 0 {
65+
let queue: Box<Queue> = Box::from_raw(queue);
66+
for to_run in *queue {
67+
to_run.invoke(());
68+
}
6169
}
6270
}
6371
}
6472
}
6573

66-
pub fn push(f: Thunk<'static>) {
74+
pub fn push(f: Thunk<'static>) -> bool {
75+
let mut ret = true;
6776
unsafe {
6877
LOCK.lock();
69-
init();
70-
(*QUEUE).push(f);
78+
if init() {
79+
(*QUEUE).push(f);
80+
} else {
81+
ret = false;
82+
}
7183
LOCK.unlock();
7284
}
85+
return ret
7386
}

src/libstd/rt/mod.rs

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,9 @@
1717
//! time being.
1818
1919
#![unstable(feature = "std_misc")]
20-
21-
// FIXME: this should not be here.
2220
#![allow(missing_docs)]
2321

24-
#![allow(dead_code)]
25-
26-
use marker::Send;
27-
use ops::FnOnce;
22+
use prelude::v1::*;
2823
use sys;
2924
use thunk::Thunk;
3025
use usize;
@@ -149,13 +144,16 @@ fn lang_start(main: *const u8, argc: int, argv: *const *const u8) -> int {
149144

150145
/// Enqueues a procedure to run when the main thread exits.
151146
///
152-
/// It is forbidden for procedures to register more `at_exit` handlers when they
153-
/// are running, and doing so will lead to a process abort.
147+
/// Currently these closures are only run once the main *Rust* thread exits.
148+
/// Once the `at_exit` handlers begin running, more may be enqueued, but not
149+
/// infinitely so. Eventually a handler registration will be forced to fail.
154150
///
155-
/// Note that other threads may still be running when `at_exit` routines start
156-
/// running.
157-
pub fn at_exit<F: FnOnce() + Send + 'static>(f: F) {
158-
at_exit_imp::push(Thunk::new(f));
151+
/// Returns `Ok` if the handler was successfully registered, meaning that the
152+
/// closure will be run once the main thread exits. Returns `Err` to indicate
153+
/// that the closure could not be registered, meaning that it is not scheduled
154+
/// to be rune.
155+
pub fn at_exit<F: FnOnce() + Send + 'static>(f: F) -> Result<(), ()> {
156+
if at_exit_imp::push(Thunk::new(f)) {Ok(())} else {Err(())}
159157
}
160158

161159
/// One-time runtime cleanup.

src/libstd/sys/common/helper_thread.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ impl<M: Send> Helper<M> {
112112
self.cond.notify_one()
113113
});
114114

115-
rt::at_exit(move || { self.shutdown() });
115+
let _ = rt::at_exit(move || { self.shutdown() });
116116
*self.initialized.get() = true;
117117
} else if *self.chan.get() as uint == 1 {
118118
panic!("cannot continue usage after shutdown");

0 commit comments

Comments
 (0)