Skip to content

Race condition in inner_enqueue() causes SIGSEGV in size_approx() on AArch64 #171

Description

@cppmage

Problem

size_approx() can dereference a null block while traversing the block
chain, due to a missing release fence when publishing a newly allocated
block in inner_enqueue().

SIGSEGV reproduced on AArch64 with:

    cmake -B build -S . -DCMAKE_BUILD_TYPE=RelWithDebInfo
    cmake --build build

GDB state at the crash:

    Thread 506 "spscq_unittests" received signal SIGSEGV, Segmentation fault.

    moodycamel::ReaderWriterQueue<int, 512ul>::size_approx(...)
        at /home/library/include/readerwriterqueue.h:500

    500     size_t blockFront = block->front.load();

    (gdb) info locals
    blockFront = <optimized out>
    blockTail = <optimized out>
    result = 27636
    frontBlock_ = 0xfffff00058f0
    block = 0x0

frontBlock_ is valid; block became nullptr while walking Block::next.

Root cause:

In inner_enqueue()'s CanAlloc branch, a newly allocated block
is published into the block chain via tailBlock_->next = newBlock; BEFORE
fence(memory_order_release); is executed - the fence currently only
precedes tailBlock = newBlock;:

newBlock->next = tailBlock_->next.load();
tailBlock_->next = newBlock;      // published here, no preceding release
...
fence(memory_order_release);      // too late for the line above
tailBlock = newBlock;

size_approx() reaches new blocks purely by walking Block::next - it
never reads tailBlock. On a strongly-ordered architecture (x86/x64) this
is masked because stores retire in program order, so the bug doesn't
surface. On a weakly-ordered architecture (AArch64) the store to
tailBlock_->next can become visible to another core before the writes
that initialize *newBlock (e.g. newBlock->next, left at its default
value otherwise), so a concurrent size_approx() can chase the new next
pointer into a block whose own next field hasn't propagated yet, read a
stale/default nullptr, and dereference it on the following iteration.

Fix:

Move the release fence before both publishing writes, so any thread
observing either tailBlock_->next == newBlock or tailBlock == newBlock
is guaranteed to see the fully initialized block. PR attached with the fix
and an updated comment explaining why size_approx() needs this in addition
to try_dequeue().

Reproduction environment note: I built via a custom cross-platform
CMakeLists.txt (the upstream project ships an MSBuild/VS2010 project);
functionally this doesn't change the source, only the build tooling.
"Reproducible" here means: running the existing size_approx stress test
in a loop (1000 iterations) reliably reproduced the SIGSEGV within that
run; I did not use a separate fuzzing/TSAN harness beyond that.

The existing size_approx test also has other pre-existing assertion
failures; those are unrelated and not addressed by this change.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions