Feature request: make std::thread::yield_now() work

[SUPERCILEX]

Over in rust-lang/rust#117485, it'd be nice to be able to force pre-emption, but I can't seem to get miri to consistently preempt. It'd be nice if std::thread::yield_now() forced a miri thread preemption.

[RalfJung]

yield_now does force preemption in Miri.

[SUPERCILEX]

Huh, any idea why -Zmiri-preemption-rate=0 wouldn't have UB in this code then?

fn no_race_with_is_abandoned() {
    static mut V: u32 = 0;
    let (p, c) = RingBuffer::<u8>::new(7);
    let t = std::thread::spawn(move || {
        unsafe { V = 10 };
        drop(p);
    });
    std::thread::yield_now();
    if c.is_abandoned() {
        unsafe { V = 20 };
    } else {
        panic!("no");
        // This is not synchronized, both Miri and ThreadSanitizer should detect a data race:
        //unsafe { V = 30 };
    }
    t.join().unwrap();
}

Interestingly, 3 yields in a loop triggers UB:

    for _ in 0..3 {
        std::thread::yield_now();
    }

But if I write them out manually 3 times it doesn't work and I have to write 6 of them.

[SUPERCILEX]

drop(p) is just running Arc::drop with a strong count > 1 btw.

[RalfJung]

Are you sure the UB just needs the right schedule? There is other non-determinism, like the weak memory emulation. You might be just probing various random executions here.

Try running Miri with different seeds and just a single yield.

[RalfJung]

Also, that code references the undefined symbol RingBuffer. Do you have a self-contained example?

[SUPERCILEX]

Nice! A seed of 1 worked lol. Here's a self contained repro for reference:

fn no_race_with_is_abandoned() {
    static mut V: u32 = 0;
    let arc = std::sync::Arc::new(());
    let arc2 = arc.clone();
    let t = std::thread::spawn(move || {
        unsafe { V = 10 };
        drop(arc2);
    });
    std::thread::yield_now();
    if std::sync::Arc::strong_count(&arc) == 1 {
        unsafe { V = 20 };
    } else {
        panic!("no");
        // This is not synchronized, both Miri and ThreadSanitizer should detect a data race:
        //unsafe { V = 30 };
    }
    t.join().unwrap();
}

---

I still don't understand where that extra non-determinism is coming from. Here's a really neat feature idea: would it be possible to log every non-determinism choice made? Then I could hopefully get a sense of the paths being explored by which seeds.

[RalfJung]

I think you'd drown in output.^^ There's at least one non-deterministic choice made for each local variable that gets allocated (to pick its absolute address). That's a lot.

You can use -Zmiri-track-weak-memory-loads to see if maybe weak memory loads are relevant. (That would be disabled with -Zmiri-disable-weak-memory-emulation.)

Also, sometimes cross-thread address reuse leads to unintended synchronization, so you can use -Zmiri-address-reuse-cross-thread-rate=0 to disable that.

[SUPERCILEX]

I think you'd drown in output.^^ There's at least one non-deterministic choice made for each local variable that gets allocated (to pick its absolute address). That's a lot.

Ohhhhh, ok well fun idea in theory but probably not so useful in reality. :)

miri-track-weak-memory-loads

TIL! That's neat. And it was the culprit!

3304 |             Relaxed => intrinsics::atomic_load_relaxed(dst),
     |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ weak memory emulation: outdated value returned from load at 0x171bc8[alloc62564]<189401>
     |
     = note: BACKTRACE on thread `no_race_with_is`:
     = note: inside `std::sync::atomic::atomic_load::<usize>` at /home/asaveau/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/sync/atomic.rs:3304:24: 3304:60
     = note: inside `std::sync::atomic::AtomicUsize::load` at /home/asaveau/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/sync/atomic.rs:2412:26: 2412:58
     = note: inside `std::sync::Arc::<()>::strong_count` at /home/asaveau/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/sync.rs:1712:9: 1712:42
note: inside `no_race_with_is_abandoned

So basically miri made strong_count return the old value which is a good thing for other test cases, but a bummer for this test. miri-disable-weak-memory-emulation fixes it.

---

Fun stuff! Thanks for helping me get to the bottom of it.