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.
Problem
size_approx()can dereference a null block while traversing the blockchain, due to a missing release fence when publishing a newly allocated
block in
inner_enqueue().SIGSEGV reproduced on AArch64 with:
GDB state at the crash:
frontBlock_is valid;blockbecame nullptr while walkingBlock::next.Root cause:
In
inner_enqueue()'s CanAlloc branch, a newly allocated blockis published into the block chain via
tailBlock_->next = newBlock;BEFOREfence(memory_order_release);is executed - the fence currently onlyprecedes
tailBlock = newBlock;:size_approx()reaches new blocks purely by walkingBlock::next- itnever reads
tailBlock. On a strongly-ordered architecture (x86/x64) thisis masked because stores retire in program order, so the bug doesn't
surface. On a weakly-ordered architecture (AArch64) the store to
tailBlock_->nextcan become visible to another core before the writesthat initialize
*newBlock(e.g.newBlock->next, left at its defaultvalue otherwise), so a concurrent
size_approx()can chase the newnextpointer into a block whose own
nextfield hasn't propagated yet, read astale/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 == newBlockortailBlock == newBlockis 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().