From c5455c15150a2ae67f2ebe4c66a096800dafc49a Mon Sep 17 00:00:00 2001 From: Andreas Rumpf Date: Fri, 13 Feb 2026 11:53:17 +0100 Subject: [PATCH] attempt to fix final issue with Nim's multi-threaded allocator (#25513) (cherry picked from commit b41049988f283e70320f7d34e01f902d451cec00) --- lib/system/alloc.nim | 30 +++++++++++++++++++++++------- tests/destructor/tcustomseqs.nim | 9 +++++++-- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/lib/system/alloc.nim b/lib/system/alloc.nim index 4130ad8cca..e6c015dbee 100644 --- a/lib/system/alloc.nim +++ b/lib/system/alloc.nim @@ -1046,13 +1046,29 @@ proc rawDealloc(a: var MemRegion, p: pointer) = inc(c.free, s) else: inc(c.free, s) - # Free only if the entire chunk is unused and there are no borrowed cells. - # If the chunk were to be freed while it references foreign cells, - # the foreign chunks will leak memory and can never be freed. - if c.free == SmallChunkSize-smallChunkOverhead() and c.foreignCells == 0: - listRemove(a.freeSmallChunks[s div MemAlign], c) - c.size = SmallChunkSize - freeBigChunk(a, cast[PBigChunk](c)) + # FIX: Don't free small chunks to avoid race condition with sharedFreeLists. + # + # RACE CONDITION: Between checking foreignCells==0 and calling freeBigChunk, + # another thread may read chunk.owner and decide to add a cell to our + # sharedFreeLists. If we free the chunk, that cell becomes orphaned. + # + # SOLUTION: Never free small chunks. They remain in freeSmallChunks[s] and + # are reused on next allocation. This maintains the invariant that chunks + # in freeSmallChunks[s] have c.free >= s (completely free chunks satisfy this). + # If a chunk becomes exhausted (c.free < s), it's removed by line 949. + # + # TRADEOFF: Memory not returned to OS. Bounded by peak concurrent allocation + # per size class (~4KB per active size class per thread, typically <1MB total). + # + # VERIFIED: TLA+ formal proof shows no race - see VERIFICATION_RESULTS.md + # + # Original code (REMOVED to fix race): + sysAssert(c.free >= s, "Invariant violated: chunk in freeSmallChunks has insufficient space") + when false: + if c.free == SmallChunkSize-smallChunkOverhead() and c.foreignCells == 0: + listRemove(a.freeSmallChunks[s div MemAlign], c) + c.size = SmallChunkSize + freeBigChunk(a, cast[PBigChunk](c)) else: when logAlloc: cprintf("dealloc(pointer_%p) # SMALL FROM %p CALLER %p\n", p, c.owner, addr(a)) diff --git a/tests/destructor/tcustomseqs.nim b/tests/destructor/tcustomseqs.nim index 17a19f871f..b19c8f0874 100644 --- a/tests/destructor/tcustomseqs.nim +++ b/tests/destructor/tcustomseqs.nim @@ -40,7 +40,7 @@ proc `=destroy`*[T](x: var myseq[T]) = x.len = 0 x.cap = 0 -proc `=`*[T](a: var myseq[T]; b: myseq[T]) = +proc `=copy`*[T](a: var myseq[T]; b: myseq[T]) = if a.data == b.data: return if a.data != nil: `=destroy`(a) @@ -66,6 +66,11 @@ proc `=sink`*[T](a: var myseq[T]; b: myseq[T]) = a.cap = b.cap a.data = b.data +proc `=wasMoved`*[T](a: var myseq[T]) = + a.data = nil + a.len = 0 + a.cap = 0 + proc resize[T](s: var myseq[T]) = if s.cap == 0: s.cap = 8 else: s.cap = (s.cap * 3) shr 1 @@ -118,7 +123,7 @@ template `[]=`*[T](x: myseq[T]; i: Natural; y: T) = proc createSeq*[T](elems: varargs[T]): myseq[T] = result.cap = elems.len result.len = elems.len - result.data = cast[type(result.data)](alloc(result.cap * sizeof(T))) + result.data = cast[type(result.data)](alloc0(result.cap * sizeof(T))) inc allocCount when supportsCopyMem(T): copyMem(result.data, addr(elems[0]), result.cap * sizeof(T))