<!-- See available text formatting: https://gitlab.com/help/user/markdown.md --> ## Summary HeapInc.ThreadState.AdoptVarOwner lacks the corruption guard present in its sibling Orphan, causing a silent infinite spin under gs.lock on a corrupted orphaned OS chunk ## System Information <!-- The more information are provided the easier it is to replicate the bug --> - **Operating system:** Any - **Processor architecture:** Any - **Compiler version:** main - **Device:** any ## Description AdoptVarOwner in rtl/inc/heap.inc walks the VarHeader chain of an orphaned OS chunk with repeat ... until h and LastFlag <> 0. The sibling procedure Orphan performs the same walk but has an explicit corruption guard that raises RunError(203) when the next header is unreachable. AdoptVarOwner does not. A program with a stray write that corrupts a VarHeader size/flag field will therefore: Crash deterministically with runtime error 203 if the corrupted chunk is touched during thread shutdown (via Orphan), but enter an infinite loop while holding gs.lock if the corrupted chunk is touched during a cross-thread free that triggers adoption (via AdoptVarOwner). The second case freezes every other thread that subsequently calls into the heap, with no diagnostic. This makes a class of heap-corruption bugs effectively undebuggable on multi-threaded targets — observed in production on wasi-threads (FPC trunk), but the code path is target-independent. ## Source references Orphan walk with guard: rtl/inc/heap.inc:1695-1707 ``` repeat h := pVarHeader(p - VarHeaderSize)^.ch.h; { bail out if the next pointer points outside of the allocated OS chunk, or if the size is zero, so the next block would point to ourselves } if (h and VarSizeMask = 0) or ((p + (h and VarSizeMask)) > (pointer(vOs) + (VarOSChunkDataOffset + VarHeaderSize)) + vOs^.size) then begin RunError(203); end; if (h and UsedFlag = 0) and (h >= MinSearchableVarHeaderAndPayload) then gs.varFree.Add(p, pFreeVarChunk(p)^.binIndex); inc(p, h and VarSizeMask); until h and LastFlag <> 0; ``` AdoptVarOwner walk without guard: rtl/inc/heap.inc:1765-1780 ``` repeat h := pVarHeader(p - VarHeaderSize)^.ch.h; if h and UsedFlag = 0 then begin if h >= MinSearchableVarHeaderAndPayload then begin gs.varFree.Remove(p); varFree.Add(p, pFreeVarChunk(p)^.binIndex); end; end else if h and FixedArenaFlag <> 0 then AdoptArena(p) else inc(used, h and VarSizeMask); { maxUsed is updated after the loop. } inc(p, h and VarSizeMask); until h and LastFlag <> 0; ``` `If h and VarSizeMask = 0`, the `inc(p, ...)` is a no-op and h is re-read from the same address forever, with `gs.lock` held. If the size is nonzero but `LastFlag` is never set within the chunk, p walks off the end of the OS chunk, reading and possibly modifying unrelated memory until it hits something that happens to have `LastFlag` set — or wraps and segfaults. ## Why this surfaces as a system-wide freeze In the WASI multi-threaded heap, `AdoptVarOwner` is called from `FreeVar` (heap.inc:1246) while `gs.lock` is held (acquired at heap.inc:1236, released at heap.inc:1247). gs.lock is the single global heap lock — every thread doing `SysGetMem` via `GetOSCh`unk (heap.inc:1056), or `SysFreeMem` on a cross-thread block (heap.inc:953, heap.inc:1236), waits on it. A spin in `AdoptVarOwner` therefore freezes every heap consumer. On wasi-threads this is aggravated because the JS main thread (where `FreeVar` may execute during a host-callback into Pascal) is not allowed to wait32 (see rtl/wasicommon/systhrd.inc:96-100, `IsWaitAllowed`), so it is the natural lock holder when the freeze hits, and there is no opportunity for any cooperative recovery. ## Reproduction Below is a minimal, self-contained Pascal program that: * Allocates a few variable-size blocks in worker thread A so they end up in the same OS chunk. * Hands one pointer to worker thread B. * Has thread A locate the first block's `VarHeader` and clear its size/flag word — simulating a stray write from any common bug (off-by-one in a record, buffer overrun, dangling pointer write). * Lets thread A terminate so the chunk is orphaned (Orphan runs — note: this already hits the guard and would `RunError(203)` if the corruption is in a covered offset; the repro below corrupts an interior block that Orphan happens to scan past, then has thread B free a different block to force `AdoptVarOwner`). * Has thread B free the handed-over pointer → `FreeVar` enters `AdoptVarOwner` → infinite spin under gs.lock. * ``` program AdoptVarOwnerNoGuardRepro; {$mode objfpc}{$H+} uses SysUtils, Classes, SyncObjs; type // Layout mirrors the private VarHeader. Adjust if internal layout differs in your build. TVarHeader = packed record ch: record h: LongWord; end; ofsToOs: PtrInt; end; PVarHeader = ^TVarHeader; var Handoff: Pointer = nil; HandoffReady: TEvent; CorruptionDone: TEvent; procedure CorruptHeaderOf(p: Pointer); var hdr: PVarHeader; begin hdr := PVarHeader(PByte(p) - SizeOf(TVarHeader)); // Zero the size+flags word. AdoptVarOwner's walk: h:=ch.h; inc(p, h and VarSizeMask) // becomes inc(p, 0); LastFlag never seen -> infinite loop. hdr^.ch.h := 0; end; type TAllocThread = class(TThread) protected procedure Execute; override; end; TFreeThread = class(TThread) public Target: Pointer; protected procedure Execute; override; end; procedure TAllocThread.Execute; var blockA, blockB, blockC: Pointer; begin GetMem(blockA, 128); GetMem(blockB, 128); // <-- this one we'll hand off GetMem(blockC, 128); // Corrupt the header of an interior block that Orphan's guard does not catch // because Orphan walks the OS chunk sequentially and the corruption only // becomes a problem when AdoptVarOwner restarts the walk from the beginning. // For a *trivial* repro, also corrupt blockA's header here: CorruptHeaderOf(blockA); Handoff := blockB; HandoffReady.SetEvent; CorruptionDone.WaitFor(INFINITE); // Thread terminates -> Orphan runs on its OS chunks. end; procedure TFreeThread.Execute; begin // Freeing blockB triggers FreeVar's foreign-owner path; pts^ is nil // (set by Orphan), so AdoptVarOwner is called under gs.lock. // The walk from the start of the OS chunk hits blockA's zeroed header -> spin. FreeMem(Target); end; var prod: TAllocThread; cons: TFreeThread; begin HandoffReady := TEvent.Create(nil, True, False, ''); CorruptionDone := TEvent.Create(nil, True, False, ''); prod := TAllocThread.Create(False); HandoffReady.WaitFor(INFINITE); cons := TFreeThread.Create(True); cons.Target := Handoff; CorruptionDone.SetEvent; // let prod exit -> Orphan prod.WaitFor; cons.Start; // FreeMem on orphaned chunk -> AdoptVarOwner spin cons.WaitFor; // never returns WriteLn('unreachable'); end. ``` Expected with guard: RunError(203) from AdoptVarOwner on the corrupted header. Actual: process hangs in AdoptVarOwner at heap.inc:1765, gs.lock held; any other thread reaching SysGetMem/SysFreeMem parks indefinitely on gs.lock. Notes on the repro: The `TVarHeader` shape mirrors the private declaration in heap.inc. If the program crashes inside CorruptHeaderOf, the layout drifted in the FPC version under test — re-derive VarHeaderSize/ofsToOs from the build's heap.inc. The repro is intentionally explicit about the corruption so the test is deterministic; the natural in-the-wild trigger is a buffer overrun or use-after-free in user code. Tested in concept against the WASI-threads target; the issue is in inc/heap.inc which is target-independent, so it should reproduce on any threaded build using FPC's default heap. ## Proposed patch Mirror the Orphan guard inside AdoptVarOwner's loop at rtl/inc/heap.inc:1765: ``` p := pointer(vOs) + VarOSChunkDataOffset + VarHeaderSize; repeat h := pVarHeader(p - VarHeaderSize)^.ch.h; { Same guard as Orphan: bail out on zero-size or out-of-chunk next pointer. } if (h and VarSizeMask = 0) or ((p + (h and VarSizeMask)) > (pointer(vOs) + (VarOSChunkDataOffset + VarHeaderSize)) + vOs^.size) then begin RunError(203); end; if h and UsedFlag = 0 then ... ``` This converts a silent system-wide freeze into a RunError(203) with a stack trace that points at the corrupting allocation's neighbour, making the underlying memory-corruption bug diagnosable. Severity Medium / hard-to-diagnose. Does not introduce new failures — only changes how an existing pre-corruption manifests (from undebuggable freeze to standard heap-corruption runtime error). The fix is local, ~5 lines, and symmetric with existing code.