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

diff --git a/src/libnative/task.rs b/src/libnative/task.rs
index ddfd46ecad9b5bc9fa07d0c3c7850d7aa20cf50e..8a82ae55faa37554a127028d91377a3e6ac180ab 100644 (file)
--- a/src/libnative/task.rs
+++ b/src/libnative/task.rs
@@ -201,19 +201,30 @@ fn deschedule(mut ~self, times: uint, mut cur_task: ~Task,
                     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();
                 }
             }

diff --git a/src/test/run-pass/issue-13494.rs b/src/test/run-pass/issue-13494.rs
new file mode 100644 (file)
index 0000000..84da2d8
--- /dev/null
+++ b/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();
+}