Skip to content

Commit d4de99a

Browse files
committed
std::rt: Fix a race in the UvRemoteCallback dtor
1 parent d6ccc6b commit d4de99a

File tree

1 file changed

+15
-11
lines changed

1 file changed

+15
-11
lines changed

src/libstd/rt/uv/uvio.rs

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use rt::sched::Scheduler;
2424
use rt::io::{standard_error, OtherIoError};
2525
use rt::tube::Tube;
2626
use rt::local::Local;
27-
use unstable::sync::{UnsafeAtomicRcBox, AtomicInt};
27+
use unstable::sync::{Exclusive, exclusive};
2828

2929
#[cfg(test)] use container::Container;
3030
#[cfg(test)] use uint;
@@ -105,21 +105,20 @@ fn test_callback_run_once() {
105105
pub struct UvRemoteCallback {
106106
// The uv async handle for triggering the callback
107107
async: AsyncWatcher,
108-
// An atomic flag to tell the callback to exit,
109-
// set from the dtor.
110-
exit_flag: UnsafeAtomicRcBox<AtomicInt>
108+
// A flag to tell the callback to exit, set from the dtor. This is
109+
// almost never contested - only in rare races with the dtor.
110+
exit_flag: Exclusive<bool>
111111
}
112112

113113
impl UvRemoteCallback {
114114
pub fn new(loop_: &mut Loop, f: ~fn()) -> UvRemoteCallback {
115-
let exit_flag = UnsafeAtomicRcBox::new(AtomicInt::new(0));
115+
let exit_flag = exclusive(false);
116116
let exit_flag_clone = exit_flag.clone();
117117
let async = do AsyncWatcher::new(loop_) |watcher, status| {
118118
assert!(status.is_none());
119119
f();
120-
let exit_flag_ptr = exit_flag_clone.get();
121-
unsafe {
122-
if (*exit_flag_ptr).load() == 1 {
120+
do exit_flag_clone.with_imm |&should_exit| {
121+
if should_exit {
123122
watcher.close(||());
124123
}
125124
}
@@ -139,9 +138,14 @@ impl Drop for UvRemoteCallback {
139138
fn finalize(&self) {
140139
unsafe {
141140
let this: &mut UvRemoteCallback = cast::transmute_mut(self);
142-
let exit_flag_ptr = this.exit_flag.get();
143-
(*exit_flag_ptr).store(1);
144-
this.async.send();
141+
do this.exit_flag.with |should_exit| {
142+
// NB: These two things need to happen atomically. Otherwise
143+
// the event handler could wake up due to a *previous*
144+
// signal and see the exit flag, destroying the handle
145+
// before the final send.
146+
*should_exit = true;
147+
this.async.send();
148+
}
145149
}
146150
}
147151
}

0 commit comments

Comments
 (0)