From bfbcfb0489e2b52657ceb117586fca4b6a89924e Mon Sep 17 00:00:00 2001 From: Andrew Haines Date: Wed, 24 Jun 2026 18:16:14 -0400 Subject: [PATCH] fix(stdlib): release managed elements in generics.collections TList MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TList stored its elements in a raw GetMem buffer and freed only the buffer (Destroy did FreeMem with no element cleanup; Clear just reset the count; Delete's shift left a duplicate ref on the vacated tail slot). For a managed element type — a class (ARC) or a string — that leaked every contained value: destroying or clearing the list never released its items, so their destructors never ran. Release each managed element by storing a zero-initialised T through its slot before dropping it: the compiler's ARC discipline on the managed store releases the previous value, so freeing the list cascades to its items. For a non-managed T (e.g. Integer) the same store is a harmless zero write, so TList is unaffected. Applied to Destroy (all elements), Clear (all elements), and Delete (the vacated tail slot). The sibling containers (TStack, TQueue, TSet, and the dictionaries) share the same raw-buffer pattern and leak likewise; left as a follow-up. Test: an e2e regression on both backends asserts a class element's destructor fires on Clear and on Free (the cascade), and the existing TList coverage confirms the non-managed path is unchanged. --- .../src/test/pascal/cp.test.e2e.tlist.pas | 41 +++++++++++++++++++ .../src/main/pascal/generics.collections.pas | 40 ++++++++++++++++-- 2 files changed, 77 insertions(+), 4 deletions(-) diff --git a/compiler/src/test/pascal/cp.test.e2e.tlist.pas b/compiler/src/test/pascal/cp.test.e2e.tlist.pas index e380d40..712fd59 100644 --- a/compiler/src/test/pascal/cp.test.e2e.tlist.pas +++ b/compiler/src/test/pascal/cp.test.e2e.tlist.pas @@ -38,6 +38,10 @@ type { Default array property: List[i] subscript for read and write. } procedure TestRun_TList_DefaultProperty_ReadWrite; procedure TestRun_TList_DefaultProperty_Polymorphic; + + { Clear and Free release managed elements (ARC cascade): a class element's + destructor must run when the list is cleared or freed. } + procedure TestRun_TList_FreeAndClear_ReleasesElements; end; implementation @@ -274,6 +278,43 @@ begin AssertEquals('List[i].Area (native)', '25' + #10, Output); end; +const + SrcTListFreeClear = ''' + program P; + uses generics.collections; + type + TC = class + N: Integer; + constructor Create(AN: Integer); + destructor Destroy; override; + end; + constructor TC.Create(AN: Integer); + begin N := AN end; + destructor TC.Destroy; + begin WriteLn('d', N) end; + var + L: TList; + begin + L := TList.Create(); + L.Add(TC.Create(1)); + L.Add(TC.Create(2)); + L.Clear(); + WriteLn('cleared'); + L.Add(TC.Create(3)); + L.Free(); + WriteLn('done') + end. + '''; + +procedure TE2ETListTests.TestRun_TList_FreeAndClear_ReleasesElements; +begin + if not ToolchainAvailable() then begin Ignore('toolchain unavailable'); Exit; end; + { Clear releases elements 1 and 2; the final Free releases element 3 — each + destructor fires, proving the managed-element release cascade. } + AssertRTLRunsOnAll(SrcTListFreeClear, + 'd1' + #10 + 'd2' + #10 + 'cleared' + #10 + 'd3' + #10 + 'done' + #10, 0); +end; + initialization RegisterTest(TE2ETListTests); diff --git a/stdlib/src/main/pascal/generics.collections.pas b/stdlib/src/main/pascal/generics.collections.pas index 7c393dd..6199db5 100644 --- a/stdlib/src/main/pascal/generics.collections.pas +++ b/stdlib/src/main/pascal/generics.collections.pas @@ -302,9 +302,10 @@ end; procedure TList.Delete(AIndex: Integer); var - Src: ^T; - Dst: ^T; - I: Integer; + Src: ^T; + Dst: ^T; + I: Integer; + Empty: T; begin I := AIndex; while I < Self.FCount - 1 do @@ -314,16 +315,47 @@ begin Dst^ := Src^; I := I + 1 end; - Self.FCount := Self.FCount - 1 + Self.FCount := Self.FCount - 1; + { The shift leaves the (now unused) tail slot holding a duplicate of the last + element; clear it so its managed ref is released, not leaked. } + Dst := Self.FData + Self.FCount * SizeOf(T); + Dst^ := Empty end; procedure TList.Clear; +var + I: Integer; + Slot: ^T; + Empty: T; begin + { Release each managed element before dropping the count (see Destroy). } + I := 0; + while I < Self.FCount do + begin + Slot := Self.FData + I * SizeOf(T); + Slot^ := Empty; + I := I + 1 + end; Self.FCount := 0 end; procedure TList.Destroy; +var + I: Integer; + Slot: ^T; + Empty: T; begin + { Release each managed element so freeing the list cascades to its items + (Blaise is reference-counted): storing a zero-initialised T through the + slot runs the compiler's ARC discipline — the old element is released; for + a plain (non-managed) T the store is a harmless zero write. } + I := 0; + while I < Self.FCount do + begin + Slot := Self.FData + I * SizeOf(T); + Slot^ := Empty; + I := I + 1 + end; FreeMem(Self.FData); Self.FData := nil; Self.FCount := 0;