Miri hangs in spin loops forever #1388
Comments
RalfJung
added
C-bug
|
We could do round robin, where every terminator causes the next thread to get activated. |
|
Ah turns out there is a stable version of the same intrinsic,
I was more thinking "switch every N terminators", but N = 1 is certainly a possible choice. ;) We might make this configurable even. |
|
There's an interesting testcase in once_cell where even adding a spin loop hint does not help. My guess is that the scheduler keeps swapping between two of the spinning threads so that the others are starved. |
|
I think here is another example where I assume @JCTyblaidd's PR #1651 would help here. |
This is the most likely scenario because the threads with the lower ID get priority (simply because we pick them by traversing the list). |
|
Got bitten by this recently when I forgot to call I think it'd be useful to document this for now, I can send a PR if you agree. Anyway, thanks for your great work on this tool! I feel much better now that I see my |
Seems better to use a different scheduling algorithm instead. One version of that started in #1651, but was not completed. A simpler alternative would be a round-robin scheduler. Until that happens, I agree documenting this better would be good, and a PR would be definitely welcome. :) |
|
Sorry for being unclear. It seems to me that a spinloop without |
Hm, that might be the case (not sure if everyone would agree) but I don't think Miri is necessarily the tool for detecting such things. None of the solutions I know of that would make Miri better at running such code would provide any avenue towards having such a warning. |
|
Sure, thanks! |
Document threading support a bit more This adds a few known limitations around threading to the README and suggests the users to look into GitHub issues to learn more. Addresses #1388 (comment)
831: Add spin loop hints in tests for Miri r=taiki-e a=cbeuw This is a better way to do #829 Miri does not have a pre-emptive scheduler, so once the execution falls into a spin loop it'll hang forever: rust-lang/miri#1388 Similar measures (`yield_now()`) are already present in [some other tests](https://github.com/crossbeam-rs/crossbeam/blob/master/crossbeam-queue/tests/array_queue.rs), but it's missing here 832: Fix links to macro rules in docs r=taiki-e a=alygin The fix provides explicit links to macro rules in docs because they [are not resolved globally anymore](rust-lang/rust#96676). Co-authored-by: Andy Wang <[email protected]> Co-authored-by: Andrew Lygin <[email protected]>
Here's a self-contained testcase for that situation: use std::thread;
use std::sync::atomic::{AtomicUsize, Ordering};
static FLAG: AtomicUsize = AtomicUsize::new(0);
// When a thread yields, Miri's scheduler will pick the thread with the lowest ID
// that can run. IDs are assigned in thread creation order.
// This means we can make 2 threads 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();
} |
|
#2197 helps with that. So after that PR, at least explicitly yielding spinloops should always work in Miri. |
That test (originally by @jethrogb) also works now, but I had to change it a little. Unchanged, it double-panics: I think this might be a true positive; that code does call |
make Miri's scheduler proper round-robin When thread N blocks or yields, we activate thread N+1 next, rather than always activating thread 0. This should guarantee that as long as all threads regularly yield, each thread eventually takes a step again. Fixes the "multiple loops that yield playing ping-pong" part of #1388. `@cbeuw` I hope this doesn't screw up the scheduler-dependent tests you are adding in your PR.
|
@RalfJung The code with rx.recv had other issues, the final test that was upstreamed doesn't use MPSC channels, see rust-lang/rust#84409 |
|
Ah, that looks like a nice test, I'll add it to Miri. :) |
The following program loops forever in Miri, but terminates as expected with normal rustc:
One could argue the spin loop should use
hint::spin_loop();, but that is unstable -- and it seems reasonable to expect the scheduler to de-schedule a thread at some point even if it does not yield.Cc @vakaras
The text was updated successfully, but these errors were encountered: