Use loom checker?

[Restioson]

Hi, thanks for writing this crate :) At first glance, it seems that the tests are a little basic, so maybe it would be good to use loom to check the correctness of the RwLock's inner atomic operations? I don't know how this interacts with lock_api at all.

[Restioson]

This may be useful? tokio-rs/loom#162

[notgull]

Thank you for your issue. I didn't know about loom prior to this. I'm going to rewrite this crate using a loom feature. Give me some time.

It shouldn't interact with lock_api at all. lock_api only acts as a wrapper around the lock that also contains the associated data.

[Restioson]

In that case, there needs to just be a kind of facade to abstract over std and loom types. I implemented this for concurrent-queue in smol-rs/concurrent-queue#5. For spinny, it seems like the required modification would simply be to replace this

use core::sync::atomic::{spin_loop_hint, AtomicUsize, Ordering};

with this

#[cfg(not(feature = "loom"))]
use core::sync::atomic::{spin_loop_hint, AtomicUsize, Ordering};
#[cfg(feature = "loom")]
use loom::sync::atomic::{spin_loop_hint, AtomicUsize, Ordering};

[notgull]

This does actually have an issue, actually. This code presently relies on the fact that AtomicUsize can be initialized in constant contexts. This line:

const INIT: RawRwSpinlock = RawRwSpinlock(AtomicUsize::new(0));

causes a compilation error, as loom's AtomicUsize cannot be initialized here. The alternative to this is to have the RawRwSpinlock be this:

pub struct RawRwSpinlock(Option<AtomicUsize>);

and then initialize the spinlock in the lock functions. However, the lock functions use immutable access, which means that in order to change the value of the Option at runtime, we need to wrap it in a primitive that allows for interior mutability:

pub struct RawRwSpinlock(UnsafeCell<Option<AtomicUsize>>);

However, this can cause a data race if two users try to initialize the spinlock at once. I'm not sure if there's an easy solution to this.

[Restioson]

Hmm... that seems to be a problem. I did find tokio-rs/loom#161 so maybe this could be helpful?

[notgull]

Using a OnceCell seems like an alright solution, except for the fact that I'm essentially just putting a mutex inside of my mutex. Let me try this.

[Restioson]

Hmmm... When does this need to be locked?

Also, this could be cfg'd out only to happen with loom on. If this only need to be locked upon initialization then maybe it would still be useful for correctness checking?