attempt to fix final issue with Nim's multi-threaded allocator (#25513)

(cherry picked from commit b41049988f)
This commit is contained in:
Andreas Rumpf
2026-02-13 11:53:17 +01:00
committed by narimiran
parent 334f2d6a87
commit c5455c1515
2 changed files with 30 additions and 9 deletions

View File

@@ -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))

View File

@@ -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))