Atomics and Locks Chapter 4 example clarification

[kavirajk]

In chapter 4, The minimal implementation example shows simplest spinlock you could built and use (with some unsafe code on the client side).

This is the exact example.

When I try to use it to access mutable static data on different threads, I get in-consistent result (data race)

Example.

use std::{
    sync::atomic::{AtomicBool, Ordering},
    thread,
};

static mut DATA: i32 = 0;

pub struct SpinLock {
    locked: AtomicBool,
}

impl SpinLock {
    pub const fn new() -> Self {
        Self {
            locked: AtomicBool::new(false),
        }
    }

    pub fn lock(&self) {
        while !self.locked.swap(true, Ordering::Acquire) {
            std::hint::spin_loop();
        }
    }

    pub fn unlock(&self) {
        self.locked.store(false, Ordering::Release);
    }
}

fn main() {
    let lock = SpinLock::new();

    thread::scope(|s| {
        for _ in 0..100 {
            s.spawn(|| {
                lock.lock();
                unsafe {
                    DATA += 1;
                }
                lock.unlock();
            });
        }
    });

    assert_eq!(unsafe { DATA }, 100);
}

When I run this program enough times, I get inconsistent result, the final assertion fails.

for example

seq 200 | xargs -I{} cargo run

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `99`,
 right: `100`', src/main.rs:45:5

My understanding is, even though we have to use some unsafe code to actually mutate the locked data, the unlock() and lock() methods are still establishing happens-before relationship just fine with Acquire and Store ordering.

And I would expect, data access is to be synchronized correctly. But output says otherwise.

Not sure what am I missing here? Can someone help? Thanks in advance.

[jessa0]

From the description of Acquire-Release Ordering (emphasis mine)

... All writes in the current thread are visible in other threads that acquire the same atomic variable.

Your main thread doesn't ever acquire the lock, so writes from the other threads are not guaranteed to be visible to it. Maybe try locking for the final read of that variable?

[user16251]

I am curious: what CPU are you using to run your code?

[kavirajk]

Your main thread doesn't ever acquire the lock, so writes from the other threads are not guaranteed to be visible to it. Maybe try locking for the final read of that variable?

Good point. I think we don't need lock on the main thread just because, I have used scoped thread here, meaning all the thread will be joined() when scope s ends, which is even before the final assert.

And I believe we always have "happens-before" relationship between the joined thread and what happens after the join call. According to this in the book

Similarly, joining a thread creates a happens-before relationship between the joined thread and what happens after the join() call.

Having said all that. I also tried accessing the data in the final assert with lock() and unlock() Still assertion fails if it runs enough times.

[user16251]

Why would the equivalent code with Mutex::into_inner (or Mutex::get_mut)? work?

[kavirajk]

I am curious: what CPU are you using to run your code?

rustplay2$ uname -m
x86_64

[geeklint]

I think the loop condition in lock() is backwards, since swap will return the previous value, it should only exit the loop when swap returns false (the lock was previously unlocked, we locked it), but right now it exits the loop when it returns true.

The code in this section does not have the !:

https://marabos.nl/atomics/building-spinlock.html#a-minimal-implementation

[kavirajk]

Great. thanks @geeklint for finding this. should have diffed my version with book's version.

And thanks for the explanation.