Custom SpinLock implementation

by

Summary

A developer attempted to implement a custom SpinLock in Rust based on Mara Bos’s book to protect shared mutable state across threads. The code resulted in memory safety issues (segmentation faults or undefined behavior) because the raw SpinLock was used to guard an UnsafeCell containing a String without adhering to Rust’s strict concurrency safety rules. Specifically, the SpinLock implementation lacked an unsafe impl<T: Send> Sync for SpinLock<T> guarantee, and the usage site failed to enforce Send bounds or proper synchronization semantics for the UnsafeCell.

Root Cause

The immediate cause of the crash is unsafe synchronization of a non-Send type (specifically, an UnsafeCell<String> not properly wrapped) and missing Sync implementation for the custom lock.

  1. Missing Sync for the Lock: The SpinLock struct defined in the module does not implement Sync. While AtomicBool is Sync, the lock primitive itself needs to be explicitly marked Sync to allow sharing across threads via &SpinLock. Without this, std::thread::scope cannot safely pass a reference to it.
  2. Improper UnsafeCell Usage: The SyncWrapper attempts to make UnsafeCell<String> Sync. However, simply wrapping UnsafeCell and adding unsafe impl Sync is a blanket promise to the compiler. If that promise is broken (e.g., the underlying data is not Send or the synchronization primitive is insufficient), you get undefined behavior.
  3. Type Constraints: String is Send + Sync. However, UnsafeCell<T> is not Sync (because it allows interior mutability). The SyncWrapper bypasses this, but the SpinLock implementation provided was too primitive to correctly manage the Drop and Send semantics of String in a panic-safe manner, leading to potential data races or double-frees.

Why This Happens in Real Systems

Engineers often “cargo cult” synchronization primitives from low-level languages (C/C++) into Rust, assuming AtomicBool + spin_loop is enough.

  • The “It Compiles” Fallacy: Rust’s strict compiler blocks safe concurrency bugs. When engineers use unsafe, they bypass these checks. If they get the unsafe logic wrong (like missing bounds), the compiler generates code that assumes the safety rules were followed.
  • Low-Level Optimizations: Developers implement spinlocks to avoid OS overhead. However, spinlocks are notoriously hard to get right in safe Rust because they require precise memory ordering and thread parking logic.
  • Undefined Behavior is Silent: Unlike a panic, unsafe concurrency bugs (data races) do not always crash immediately. They cause silent corruption or random crashes later, making them hard to debug.

Real-World Impact

  • Segmentation Faults: The most likely symptom of this code is an immediate segfault when the threads try to push to the String inside UnsafeCell, because the memory layout is being accessed incorrectly or the String pointer is dangling.
  • Data Races: Even if it runs, two threads might overwrite memory locations if the Acquire/Release ordering is insufficient (though swap with Acquire/Release is usually sufficient for a simple lock, the lack of Sync prevents the compiler from verifying thread safety).
  • Deadlocks: While this specific example uses a timeout, naive spinlocks often fail to implement proper futex or thread parking, leading to CPU spinning and system unresponsiveness under load.

Example or Code (if necessary and relevant)

To fix the SpinLock to be usable and safe, it must properly handle thread wake-up (futex) and implement Sync/Send correctly. Below is a corrected version of the SpinLock that uses parking_lot logic or standard library primitives to be actually useful, rather than just spinning. Note: The original code’s UnsafeCell usage was incorrect; the fix requires wrapping the data in Mutex or implementing the lock correctly to own the data.

Here is a valid, working implementation of a SpinLock (using std::sync::atomic and thread::yield_now for portability, though parking_lot is preferred in production):

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

pub struct SpinLock {
    locked: AtomicBool,
}

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

    pub fn lock(&self) {
        // Strict memory ordering is crucial.
        while self
            .locked
            .swap(true, Ordering::Acquire)
        {
            // Ideally, use thread::park() here, but a spin loop with yield is:
            thread::yield_now();
        }
    }

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

// CRITICAL: Without this, you cannot share &SpinLock across threads.
unsafe impl Sync for SpinLock {}
unsafe impl Send for SpinLock {}

How Senior Engineers Fix It

Senior engineers avoid custom synchronization primitives unless absolutely necessary.

  1. Use Standard Primitives: Replace the custom SpinLock with std::sync::Mutex. It handles OS-level futex waiting, poisoning, and correct memory ordering automatically.
  2. Encapsulate Unsafe: If a spinlock is needed for performance, they wrap it in a safe API. They ensure the UnsafeCell is private and the struct guarantees Sync only if it holds Send data.
  3. Use parking_lot: In the Rust ecosystem, the parking_lot crate provides Mutex and RwLock that are faster, smaller, and don’t require poisoning, avoiding the pitfalls of std::sync and raw spinlocks.
  4. Prevent UnsafeCell Leakage: They would not expose UnsafeCell<String>. Instead, they would define a struct that contains the lock and the data, ensuring that access to the data always happens through the lock.

Why Juniors Miss It

  • Misunderstanding Send and Sync: Juniors often think “shared state” just means a reference and a lock. They don’t realize that Send moves ownership between threads, while Sync allows shared references (&T) between threads.
  • The “It Just Works” Illusion: They copy a minimal AtomicBool lock example from a blog or book without understanding that the example was likely a teaching tool on atomics, not a production-ready lock.
  • Ignoring UnsafeCell Invariants: They treat UnsafeCell as a magic bypass for mut requirements, forgetting that it removes compiler protection entirely. They assume the lock makes it safe, but without the compiler enforcing the lock usage (via a safe wrapper), it is easy to forget to lock it.