Skip to content

Commit 87df504

Browse files
committed
Carefully destroy channels at the right time.
When a channel is destroyed, it may attempt scheduler operations which could move a task off of it's I/O scheduler. This is obviously a bad interaction, and some finesse is required to make it work (making destructors run at the right time). Closes #10375
1 parent 66a7f8f commit 87df504

File tree

2 files changed

+54
-5
lines changed

2 files changed

+54
-5
lines changed

src/librustuv/signal.rs

+26
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,29 @@ impl Drop for SignalWatcher {
7777
self.close_async_();
7878
}
7979
}
80+
81+
#[cfg(test)]
82+
mod test {
83+
use super::*;
84+
use std::cell::Cell;
85+
use super::super::local_loop;
86+
use std::rt::io::signal;
87+
use std::comm::{SharedChan, stream};
88+
89+
#[test]
90+
fn closing_channel_during_drop_doesnt_kill_everything() {
91+
// see issue #10375, relates to timers as well.
92+
let (port, chan) = stream();
93+
let chan = SharedChan::new(chan);
94+
let _signal = SignalWatcher::new(local_loop(), signal::Interrupt,
95+
chan);
96+
97+
let port = Cell::new(port);
98+
do spawn {
99+
port.take().try_recv();
100+
}
101+
102+
// when we drop the SignalWatcher we're going to destroy the channel,
103+
// which must wake up the task on the other end
104+
}
105+
}

src/librustuv/timer.rs

+28-5
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,8 @@ impl RtioTimer for TimerWatcher {
100100
}
101101
}
102102

103-
extern fn timer_cb(handle: *uvll::uv_timer_t, _status: c_int) {
103+
extern fn timer_cb(handle: *uvll::uv_timer_t, status: c_int) {
104+
assert_eq!(status, 0);
104105
let timer: &mut TimerWatcher = unsafe { UvHandle::from_uv_handle(&handle) };
105106

106107
match timer.action.take_unwrap() {
@@ -118,16 +119,24 @@ extern fn timer_cb(handle: *uvll::uv_timer_t, _status: c_int) {
118119

119120
impl Drop for TimerWatcher {
120121
fn drop(&mut self) {
121-
let _m = self.fire_homing_missile();
122-
self.action = None;
123-
self.stop();
124-
self.close_async_();
122+
// note that this drop is a little subtle. Dropping a channel which is
123+
// held internally may invoke some scheduling operations. We can't take
124+
// the channel unless we're on the home scheduler, but once we're on the
125+
// home scheduler we should never move. Hence, we take the timer's
126+
// action item and then move it outside of the homing block.
127+
let _action = {
128+
let _m = self.fire_homing_missile();
129+
self.stop();
130+
self.close_async_();
131+
self.action.take()
132+
};
125133
}
126134
}
127135

128136
#[cfg(test)]
129137
mod test {
130138
use super::*;
139+
use std::cell::Cell;
131140
use std::rt::rtio::RtioTimer;
132141
use super::super::local_loop;
133142

@@ -188,4 +197,18 @@ mod test {
188197
let _timer = TimerWatcher::new(local_loop());
189198
fail!();
190199
}
200+
201+
#[test]
202+
fn closing_channel_during_drop_doesnt_kill_everything() {
203+
// see issue #10375
204+
let mut timer = TimerWatcher::new(local_loop());
205+
let timer_port = Cell::new(timer.period(1000));
206+
207+
do spawn {
208+
timer_port.take().try_recv();
209+
}
210+
211+
// when we drop the TimerWatcher we're going to destroy the channel,
212+
// which must wake up the task on the other end
213+
}
191214
}

0 commit comments

Comments
 (0)