Skip to content

Commit

Permalink
native: Fix a race in select()
Browse files Browse the repository at this point in the history
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
  • Loading branch information
alexcrichton committed Apr 16, 2014
1 parent 4ca7abb commit 93dc555
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 9 deletions.
29 changes: 20 additions & 9 deletions src/libnative/task.rs
Expand Up @@ -201,19 +201,30 @@ impl rt::Runtime for Ops {
Err(task) => { cast::forget(task.wake()); }
}
} else {
let mut iter = task.make_selectable(times);
let iter = task.make_selectable(times);
let guard = (*me).lock.lock();
(*me).awoken = false;
let success = iter.all(|task| {
match f(task) {
Ok(()) => true,
Err(task) => {
cast::forget(task.wake());
false

// Apply the given closure to all of the "selectable tasks",
// bailing on the first one that produces an error. Note that
// care must be taken such that when an error is occurred, we
// may not own the task, so we may still have to wait for the
// task to become available. In other words, if task.wake()
// returns `None`, then someone else has ownership and we must
// wait for their signal.
match iter.map(f).filter_map(|a| a.err()).next() {
None => {}
Some(task) => {
match task.wake() {
Some(task) => {
cast::forget(task);
(*me).awoken = true;
}
None => {}
}
}
});
while success && !(*me).awoken {
}
while !(*me).awoken {
guard.wait();
}
}
Expand Down
56 changes: 56 additions & 0 deletions src/test/run-pass/issue-13494.rs
@@ -0,0 +1,56 @@
// Copyright 2013-2014 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// This test may not always fail, but it can be flaky if the race it used to
// expose is still present.

extern crate green;
extern crate rustuv;
extern crate native;

#[start]
fn start(argc: int, argv: **u8) -> int {
green::start(argc, argv, rustuv::event_loop, main)
}

fn helper(rx: Receiver<Sender<()>>) {
for tx in rx.iter() {
let _ = tx.send_opt(());
}
}

fn test() {
let (tx, rx) = channel();
spawn(proc() { helper(rx) });
let (snd, rcv) = channel();
for _ in range(1, 100000) {
snd.send(1);
let (tx2, rx2) = channel();
tx.send(tx2);
select! {
() = rx2.recv() => (),
_ = rcv.recv() => ()
}
}
}

fn main() {
let (tx, rx) = channel();
spawn(proc() {
tx.send(test());
});
rx.recv();

let (tx, rx) = channel();
native::task::spawn(proc() {
tx.send(test());
});
rx.recv();
}

0 comments on commit 93dc555

Please sign in to comment.