Skip to content

Commit 93dc555

Browse files
committed
native: Fix a race in select()
During selection, libnative would erroneously re-acquire ownership of a task when a separate thread still had ownership of the task. The loop in select() was rewritten to acknowledge this race and instead block waiting to re-acquire ownership rather than plowing through. Closes #13494
1 parent 4ca7abb commit 93dc555

File tree

2 files changed

+76
-9
lines changed

2 files changed

+76
-9
lines changed

src/libnative/task.rs

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -201,19 +201,30 @@ impl rt::Runtime for Ops {
201201
Err(task) => { cast::forget(task.wake()); }
202202
}
203203
} else {
204-
let mut iter = task.make_selectable(times);
204+
let iter = task.make_selectable(times);
205205
let guard = (*me).lock.lock();
206206
(*me).awoken = false;
207-
let success = iter.all(|task| {
208-
match f(task) {
209-
Ok(()) => true,
210-
Err(task) => {
211-
cast::forget(task.wake());
212-
false
207+
208+
// Apply the given closure to all of the "selectable tasks",
209+
// bailing on the first one that produces an error. Note that
210+
// care must be taken such that when an error is occurred, we
211+
// may not own the task, so we may still have to wait for the
212+
// task to become available. In other words, if task.wake()
213+
// returns `None`, then someone else has ownership and we must
214+
// wait for their signal.
215+
match iter.map(f).filter_map(|a| a.err()).next() {
216+
None => {}
217+
Some(task) => {
218+
match task.wake() {
219+
Some(task) => {
220+
cast::forget(task);
221+
(*me).awoken = true;
222+
}
223+
None => {}
213224
}
214225
}
215-
});
216-
while success && !(*me).awoken {
226+
}
227+
while !(*me).awoken {
217228
guard.wait();
218229
}
219230
}

src/test/run-pass/issue-13494.rs

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// Copyright 2013-2014 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// This test may not always fail, but it can be flaky if the race it used to
12+
// expose is still present.
13+
14+
extern crate green;
15+
extern crate rustuv;
16+
extern crate native;
17+
18+
#[start]
19+
fn start(argc: int, argv: **u8) -> int {
20+
green::start(argc, argv, rustuv::event_loop, main)
21+
}
22+
23+
fn helper(rx: Receiver<Sender<()>>) {
24+
for tx in rx.iter() {
25+
let _ = tx.send_opt(());
26+
}
27+
}
28+
29+
fn test() {
30+
let (tx, rx) = channel();
31+
spawn(proc() { helper(rx) });
32+
let (snd, rcv) = channel();
33+
for _ in range(1, 100000) {
34+
snd.send(1);
35+
let (tx2, rx2) = channel();
36+
tx.send(tx2);
37+
select! {
38+
() = rx2.recv() => (),
39+
_ = rcv.recv() => ()
40+
}
41+
}
42+
}
43+
44+
fn main() {
45+
let (tx, rx) = channel();
46+
spawn(proc() {
47+
tx.send(test());
48+
});
49+
rx.recv();
50+
51+
let (tx, rx) = channel();
52+
native::task::spawn(proc() {
53+
tx.send(test());
54+
});
55+
rx.recv();
56+
}

0 commit comments

Comments
 (0)