make Miri's scheduler proper round-robin

Author: RalfJung

src/thread.rs

@@ -518,16 +518,26 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
             return Ok(SchedulingAction::ExecuteTimeoutCallback);
         }
         // No callbacks scheduled, pick a regular thread to execute.
-        // We need to pick a new thread for execution.
-        for (id, thread) in self.threads.iter_enumerated() {
+        // The active thread blocked or yielded. So we go search for another enabled thread.
+        // Curcially, we start searching at the current active thread ID, rather than at 0, since we
+        // want to avoid always scheduling threads 0 and 1 without ever making progress in thread 2.
+        //
+        // `skip(N)` means we start iterating at thread N, so we skip 1 more to start just *after*
+        // the active thread. Then after that we look at `take(N)`, i.e., the threads *before* the
+        // active thread.
+        let threads = self
+            .threads
+            .iter_enumerated()
+            .skip(self.active_thread.index() + 1)
+            .chain(self.threads.iter_enumerated().take(self.active_thread.index()));
+        for (id, thread) in threads {
+            debug_assert_ne!(self.active_thread, id);
             if thread.state == ThreadState::Enabled {
-                if !self.yield_active_thread || id != self.active_thread {
-                    self.active_thread = id;
-                    if let Some(data_race) = data_race {
-                        data_race.thread_set_active(self.active_thread);
-                    }
-                    break;
+                self.active_thread = id;
+                if let Some(data_race) = data_race {
+                    data_race.thread_set_active(self.active_thread);
                 }
+                break;
             }
         }
         self.yield_active_thread = false;

tests/pass/concurrency/spin_loops.rs

@@ -0,0 +1,33 @@
+// ignore-windows: Concurrency on Windows is not supported yet.
+
+use std::thread;
+use std::sync::atomic::{AtomicUsize, Ordering};
+
+static FLAG: AtomicUsize = AtomicUsize::new(0);
+
+// When a thread yields, Miri's scheduler used to pick the thread with the lowest ID
+// that can run. IDs are assigned in thread creation order.
+// This means we could make 2 threads infinitely ping-pong with each other while
+// really there is a 3rd thread that we should schedule to make progress.
+
+fn main() {
+    let waiter1 = thread::spawn(|| {
+        while FLAG.load(Ordering::Acquire) == 0 {
+            // spin and wait
+            thread::yield_now();
+        }
+    });
+    let waiter2 = thread::spawn(|| {
+        while FLAG.load(Ordering::Acquire) == 0 {
+            // spin and wait
+            thread::yield_now();
+        }
+    });
+    let progress = thread::spawn(|| {
+        FLAG.store(1, Ordering::Release);
+    });
+    // The first `join` blocks the main thread and thus takes it out of the equation.
+    waiter1.join().unwrap();
+    waiter2.join().unwrap();
+    progress.join().unwrap();
+}