m-ou-se / rust-atomics-and-locks

Code examples, data structures, and links from my book, Rust Atomics and Locks.
Other
1.33k stars 120 forks source link

Guard in Chapter 4 is missing PhantomData<&'a mut T> field #16

Closed KamilaBorowska closed 4 months ago

KamilaBorowska commented 1 year ago

Type of error

Serious technical mistake

Location of the error

https://marabos.nl/atomics/building-spinlock.html#building-safe-spinlock

Description of the error

The guard is missing PhantomData<&'a mut T> field. This means it's possible to write code like the following:

use std::cell::Cell;
use std::thread;
let x = SpinLock::new(Cell::new(42));
let locked = x.lock();
thread::scope(|s| {
    s.spawn(|| locked.set(1));
    s.spawn(|| locked.set(2));
});

Which is a data race as Cell is available in two threads at once.

This problem also occurs in Chapter 9 with its MutexGuard.

m-ou-se commented 1 year ago

Thanks! This is definitely an issue.

I've looked through my pile of notes to see how this happened, and found that my original example had a slightly different guard:

pub struct Guard<'a, T> {
    locked: &'a AtomicBool,
    value: &'a mut T,
}

I must have changed it to just a &'a SpinLock<T> at some point. Not sure how I (and several reviewers) missed the Sync implementation. Thanks for spotting the mistake.

m-ou-se commented 1 year ago

For now, I've added the missing Sync impl to the code snippets in this repository. I'll update the book soon.