The data race that passed every test

A lock-free SPSC ring buffer returned the correct sum on every run while containing a data race. Then a benchmark with a 2x noise floor almost convinced me the race was worth keeping. The fix was measurement, not luck.

The false sharing post ended by asking whether the write path could be disciplined from Python or needed to live in Rust. This one starts from the answer, it needs Rust. The benchmark in that post proved that isolated counters stop fighting over cache lines, but it never moved a single piece of data between threads. This post is about building the thing that does, a single producer single consumer ring buffer, and about the two ways it went wrong before it went right.

The code went wrong twice, one race that announced itself and one that stayed silent. My reading of the benchmark went wrong once, and that mistake nearly made me ship the silent one.

A ring buffer is a fixed array where a producer writes at one index and a consumer reads at another, and both indices wrap back to zero when they reach the end. Mine holds 1024 slots. The capacity is a power of two on purpose:

const MASK: usize = 1023;

1024 is a single one bit followed by ten zeros. Subtract one and you get ten ones, a solid mask. That turns the wraparound, which is really a modulo, into a single AND instruction:

let next_tail = (tail + 1) & MASK;

This only works for powers of two. A capacity of 1000 would leave holes in the mask and the index math would silently produce wrong slots. This is the same reason hash tables and page tables size things in powers of two, the modulo becomes free.

Coordination between the two threads runs on two atomic counters. The producer owns tail and advances it after writing a slot. The consumer owns head and advances it after reading one. Each side only writes its own counter and only reads the other’s. Full means the producer’s next slot would land on the consumer’s position. Empty means the counters are equal.

The buffer slots themselves are UnsafeCell<usize>, which means the compiler will not let me share the struct across threads until I claim it is safe:

unsafe impl Sync for RingBuffer {}

That unsafe impl is a promise. The head and tail protocol guarantees the producer and consumer never touch the same slot at the same time. The compiler cannot verify that promise. Keeping it is entirely on the code, which matters for what happened next.

Every atomic operation in Rust takes an ordering, a promise about what other memory operations are allowed to move around it. Relaxed is the weakest one. The operation itself is atomic, and that is the entire guarantee, nothing about it constrains what happens before or after. The first version used Relaxed everywhere, and the habit came straight from the last post. There, Relaxed was the right choice. That benchmark protected nothing, raw write speed was the whole point, and I said so at the time. I carried it over. A counter is atomic, so no torn values, and that felt like the whole problem. A store is a store.

The correctness check is built into the benchmark itself: the producer writes the values 1 through N into the ring, the consumer sums everything it reads, and the sum has a closed form to check against. At 1 million items the expected sum is 500000500000.

The first run printed 499903981117.

That number is the memory model collecting its debt. The consumer was watching tail with a Relaxed load, so it could see the counter advance before the slot write behind it became visible, and it read slots the data had not reached yet. Not a theory, an output. Apple Silicon reorders aggressively enough to show it on the first attempt.

The fix was the pattern every example of publishing data shows, Release on the producer’s tail store, Acquire on the consumer’s tail load. The pair is a contract, everything written before a Release store is visible to any thread after its Acquire load sees the stored value. The slot write can no longer drift past the counter that announces it. The sum came back exact and stayed exact. Both the tightly packed layout and the cache line padded layout from the previous post produced the right answer every run after that, and padding was clearly faster. Everything matched the story from the false sharing post. I was ready to move on.

And that is where I stopped, because the test was green. The head channel, the mirror of the one I had just fixed, still had Relaxed on both ends:

// consumer
consumer_ring.head.store(head, Ordering::Relaxed);

// producer
while next_tail == producer_ring.head.load(Ordering::Relaxed) {

On a later pass I went through the orderings asking one question about each atomic, what message does this carry. The tail channel carries “I wrote this slot, you may read it.” The head channel carries the mirror message, “I finished reading this slot, you may overwrite it.” That store is not just a number moving. It is a permission, the same kind of message tail carries, pointed the other way, and it needs the same guarantee. Nothing in the memory model stops the consumer’s read of the slot from being reordered after its store to head. On paper, the producer can see the advanced head, write into the slot, and collide with a read that has not actually finished. That is a data race, which in Rust is undefined behavior.

The sum was still 500000500000. Every run. The test kept passing because Apple Silicon and the current compiler happened to never realize the reordering the memory model permits. The two races were never equally visible. The tail race corrupted the sum the moment anything reordered, a million opportunities per run. The head race needed a reorder and a collision inside a window of nanoseconds, and that window never happened to close. Undefined behavior does not mean wrong output. It means the program has no defined meaning, and whatever output you get is a coincidence of this compiler version on this chip. That is when it landed for me, the most dangerous bugs are the ones that pass every test you know how to write.

The fix was two words, Relaxed becomes Release on the consumer’s store and Acquire on the producer’s load. Symmetry is the rule I am taking away: every atomic that tells another thread “you may now touch that memory” is a Release store paired with an Acquire load, in both directions, no exceptions.

Here is the part I am less proud of. After the fix, the benchmark got slower, and worse, the padding advantage seemed to vanish. Some runs the padded version lost to the unpadded one. The numbers were telling me the correctness fix had cost me the very thing the last post was about.

So I reverted it. I put the race back, because the benchmark said the racy version was faster.

It took a second look to see both mistakes in that sentence. First, a benchmark of code with undefined behavior does not measure a faster algorithm. It measures whatever the optimizer did with a program that has no defined meaning. “Relaxed is faster than Acquire and Release” is only a meaningful comparison between two correct programs, and the Relaxed version was not one. The benchmark was measuring an accident.

Second, the numbers themselves were noise. The runs were 20 milliseconds long at 1 million items, launched one at a time from the shell, with run to run swings of 2x. Any single run could tell any story. I made a design decision, reverting a correctness fix, based on measurements I already knew were not trustworthy. The instrument was bad and I trusted it anyway because it agreed with what I wanted.

The fix for the instrument was boring: 10 million items so each run takes a few hundred milliseconds, ten runs per variant, compare the minimums. The minimum is the run with the least interference from everything else on the machine, and the true cost cannot be lower than the fastest observation. With clean measurement, the correct padded version beat the correct unpadded version in ten runs out of ten. The padding win had never gone anywhere. It was hiding under the noise floor of a 20 millisecond benchmark.

Whether the fix itself cost anything I never measured cleanly, the noise saw to that, and chasing the question pointed at something more useful. On ARM, Acquire and Release compile to actual barrier instructions, and they sit on the operation that was already the most expensive thing in the loop: every single iteration, each thread loaded the other thread’s counter. The producer read head before every write. The consumer read tail before every read. That load is a genuine cross core transfer, the cache line holding the counter lives in the other core’s cache in modified state, and pulling it over costs tens of nanoseconds while everything else in the loop costs almost nothing.

The question worth asking was how often each side actually needs fresh information about the other. The producer only needs to know head when the buffer might be full. With 1024 slots and a consumer keeping pace, the buffer is almost never full. Ten million loads were confirming something the producer effectively already knew.

The pattern that fixes this is a cached index. Each side keeps a local, possibly stale copy of the other side’s counter and checks that first. Only when the stale copy says full does the producer load the real atomic:

let mut tail = 0usize;
let mut cached_head = 0usize;
for i in 0..10_000_000usize {
    let next_tail = (tail + 1) & MASK;

    if next_tail == cached_head {
        cached_head = ring.head.load(Ordering::Acquire);
        while next_tail == cached_head {
            spin_loop();
            cached_head = ring.head.load(Ordering::Acquire);
        }
    }

    unsafe { *ring.buffer[tail].get() = i + 1; }
    ring.tail.store(next_tail, Ordering::Release);
    tail = next_tail;
}

The if is a comparison between two locals sitting in registers, roughly a cycle. The atomic load moved inside it, onto a path that only runs when the cached value claims the buffer is full. If the fresh load shows the consumer has moved on, the false alarm resolves with one load and the producer gets roughly another thousand silent iterations. If the buffer is genuinely full, the inner loop waits, which is the old behavior, demoted from the hot path to a slow path. The queue never stops communicating, it communicates once per lap instead of once per item, and one load gets amortized across a thousand writes the same way the mask amortizes the wraparound across the whole ring.

Staleness is safe here for one specific reason: head only ever advances. A stale copy can only underestimate the free space, never overestimate it. The worst case is an unnecessary refresh. It can never cause the producer to overwrite an unread slot. That one directional property is what makes the whole pattern legal, and it is worth checking explicitly before caching any shared counter, because a counter that could move both ways would break this silently.

The consumer gets the mirror image, a cached_tail that can only undercount available items. And the Release stores still happen every iteration on both sides. The caching removes reads of the other side’s counter, never writes to a thread’s own. Each side may listen lazily only because the other never stops announcing. Skip the store and the other side’s cache goes permanently stale.

None of this is mine. Once the per iteration loads showed up as the cost, the fix turned out to be old news, the pattern already exists in production SPSC queues under the name cached indices.

The final numbers, ten runs per variant, 10 million items each, capacity 1024, minimums reported, Apple Silicon, macOS, rustc in release mode:

unpadded, correct ordering        174.5 ms    57M items/sec
padded, correct ordering          102.8 ms    97M items/sec
padded plus cached indices         65.3 ms   153M items/sec

Padding alone is 1.7x over the packed layout, the same false sharing effect as the previous post, now visible in a structure that actually moves data. Cached indices add another 1.57x on top of padding. End to end the queue is 2.67x faster than where it started, and every version produces the same correct sum.

The spread is its own finding. The unpadded runs ranged from 174 to 245 milliseconds. The cached runs stayed between 65 and 101. False sharing does not just make code slower, it makes it erratically slower, because the penalty depends on exactly how the two threads happen to interleave. If your latency numbers are jittery and you do not know why, memory layout is worth a look before you blame the scheduler.

The numbers come with boundaries. The benchmark ran on one machine, Apple Silicon, where Acquire and Release have real hardware cost. On x86 the ordering is nearly free and the ratios would shift. The producer and consumer run at matched speeds, which is the flattering case for cached indices. If one side is much slower, the queue spends its life full or empty, the caches refresh constantly, and the win shrinks toward zero. The timer starts before buffer allocation and thread spawn, which is a rounding error at 10 million items but is still inside the measurement. And the three variants run in a fixed order in one process, so the later ones inherit a warmed up machine. None of this changes the conclusions at this effect size. All of it would matter if the gaps were small, which is exactly when I failed to account for it the first time.

What I am carrying forward is two constraints, one from the race that survived and one from the instrument.

Correct output is not correct code. The sum check earned its keep, it caught the tail race on the very first run, but it was structurally incapable of catching the mirror race that the hardware happened to tolerate. Tests observe executions. Undefined behavior is a property of the program, not of any execution you have seen. The real defenses are reasoning against the memory model and the symmetry rule, every “you may touch this memory now” signal is a Release paired with an Acquire. Tools exist that check the model rather than the output, Miri and loom, and running this queue through them is on the list.

And a benchmark is an instrument that has to be qualified before it is believed, especially when it tells you what you want to hear. Mine had a 2x noise floor and I used it to justify reverting a correctness fix. The revert felt like pragmatism in the moment. It was actually the instrument agreeing with my preference. Minimum of many runs, long enough runs to bury the jitter, and no design decisions on numbers whose spread you have not looked at.

The scheduler now has a queue that is correct by the memory model and fast by measurement, in that order. The next question is what happens when one producer is not enough, because the atomic claim design from the last post implies several, and multi producer is where the simple counter protocol stops being simple. That is the next problem to work through. This one is at least understood.

Code from this article
github.com/runbykomal

More like this