rust-atomics-and-locks icon indicating copy to clipboard operation
rust-atomics-and-locks copied to clipboard

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

Open KamilaBorowska opened this issue 1 year ago • 2 comments

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.

KamilaBorowska avatar Mar 03 '23 15:03 KamilaBorowska

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 avatar Mar 03 '23 16:03 m-ou-se

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

m-ou-se avatar Mar 03 '23 18:03 m-ou-se