SpinLock implementation

Hi,

I recently got this task at an interview: build a SpinLock using atomics and unsafe Rust.

This is my solution Rust Playground

But failed the interview without any specific feedback.

I looked online and found very similar solutions to mine, one was different as it's using sleep instead of yield, but I think the latter is better for real-time as it minimizez the wait time.

Do you think this solution is not correct? If so, how can it be made better?

Take a look at this blog post : Correctly Implementing a Spinlock in C++ | Erik Rigtorp

It's in C++, but he points out some improvements that can be made which apply to your code too.

2 Likes

Your atomic orderings are incorrect. The simplest fix is to use Acquire when locking and Release when unlocking.

I think it would be better to use spin_loop instead of yield_now, but this is dependent on what kind of spin lock you're trying to make.

The most important problem, though, is that you use false to mean "unlocked", but then break out of the loop when there is a true.

if self.lock.swap(true, Ordering::Acquire) {
    break;
}

Usually when you break out of the loop, it'll be right after the same thread swapped in a true and you won't notice any issue, but occasionally it will be right after another thread swapped in a true and you'll have 2 or more threads that think they have exclusive access.

The fix is simple.

if !self.lock.swap(true, Ordering::Acquire) {
    break;
}

Note that if you use a while loop, the condition is flipped.

while self.lock.swap(true, Ordering::Acquire) {
    std::hint::spin_loop();
}

As always, there's a whole book about atomics in Rust: https://marabos.nl/atomics/

Some other small things:

  • Storing a whole &'a SpinLock<T> inside LockGuard cuts the size of the LockGuard in half.
  • It's good practice to put comments on your unsafe code and any code the unsafe parts rely on.
  • You don't need to make the Deref impls specifically for u32.
3 Likes

thanks, yes noticed that mistake after posted
fixed it in my git code

thanks for that

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.