Generics.Collections.TObjectList does not free the item on assignment Items[I] := xxx
Original Reporter info from Mantis: Michalis @michaliskambi
-
Reporter name: Michalis Kamburelis
Original Reporter info from Mantis: Michalis @michaliskambi
- Reporter name: Michalis Kamburelis
Description:
When a list "owns" it's items, it should free the previous item on an assignment like "Items[I] := xxx". That is, calling "Items[I] := xxx" should free the previous value of the Items[I].
This is currently
- done by TObjectList from the Contnrs unit (non-generic list)
- done by FGL.TFPGObjectList (generic list)
- NOT done by the generic list TObjectList from the Generics.Collections unit.
A simple solution, following the same solution as TList.Put from rtl/objpas/classes/lists.inc, is to call a notification (about removal, and then addition) at TList&LtPos;T>.SetItem call. This will cause freeing the item by TObjectList&LtPos;T>.Notify, if ObjectsOwner = true.
I tested this patch on my copy of Generics.Collections in Castle Game Engine, and it works cool: https://github.com/castle-engine/castle-engine/commit/5e75869b834fbbab297179ba96b38ae29364a24b#diff-5d82d1b1dba6c68e79877254ea310e7d
(BTW, this solution does NOT protect you from an error of doing "Items[I] := Items[I]". This operation, that looks harmless, will actually free the item. IOW, the SetItem implementation DOES NOT check whether the old and new values are equal before doing notifications. This trap is consistent with Contnrs.TObjectList and FGL.TFPGObjectList behavior.)
Steps to reproduce:
Before fixing, the output of the attached testcase is like this:
$ fpc -gl -gh freeing_item_on_set.lpr && ./freeing_item_on_set
Free Pascal Compiler version 3.1.1-r36695 [2017/07/09] for x86_64
Copyright (c) 1993-2017 by Florian Klaempfl and others
Target OS: Linux for x86-64
....
Testing Contnrs.TObjectList:
TMyObject.Destroy
Testing FGL.TFPGObjectList:
TMyObject.Destroy
Testing Generics.Collections.TObjectList:
Heap dump by heaptrc unit
35 memory blocks allocated : 4466/4472
34 memory blocks freed : 4458/4464
1 unfreed memory blocks : 8
True heap size : 393216
True free heap : 393056
Should be : 393080
Call trace for block $00007F692463E0C0 size 8
Note that there is no "TMyObject.Destroy" after the "Testing Generics.Collections.TObjectList:" line, and there is a memory leak.
The result with patched Generics.Collections should be like this:
$ ./freeing_item_on_set
Testing Contnrs.TObjectList:
TMyObject.Destroy
Testing FGL.TFPGObjectList:
TMyObject.Destroy
Testing Generics.Collections.TObjectList:
TMyObject.Destroy
Heap dump by heaptrc unit
33 memory blocks allocated : 4234/4240
33 memory blocks freed : 4234/4240
0 unfreed memory blocks : 0
True heap size : 360448
True free heap : 360448
Mantis conversion info:
- Mantis ID: 32136
- OS: Debian GNU/Linux
- OS Build: (testing)
- Build: r36695
- Platform: x86-64
- Version: 3.1.1
- Fixed in version: 3.2.0
- Fixed in revision: 36724 (#be39ca0c)
- Target version: 3.1.1