View Issue Details

IDProjectCategoryView StatusLast Update
0032136FPCPackagespublic2017-07-11 09:38
ReporterMichalis KamburelisAssigned ToMichael Van Canneyt 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Platformx86-64OSDebian GNU/LinuxOS Version(testing)
Product Version3.1.1Product Buildr36695 
Target Version3.1.1Fixed in Version3.2.0 
Summary0032136: Generics.Collections.TObjectList does not free the item on assignment Items[I] := xxx
DescriptionWhen 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<T>.SetItem call. This will cause freeing the item by TObjectList<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 ReproduceBefore 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
TagsNo tags attached.
Fixed in Revision36724
FPCOldBugId
FPCTarget
Attached Files

Activities

Michalis Kamburelis

2017-07-11 06:52

reporter  

freeing_item_on_set.lpr (1,287 bytes)

Michael Van Canneyt

2017-07-11 09:37

administrator   ~0101660

Patched, thanks.

Michael Van Canneyt

2017-07-11 09:38

administrator   ~0101661

@Maciej, I only saw you assigned it to yourself after having fixed it..

Issue History

Date Modified Username Field Change
2017-07-11 06:52 Michalis Kamburelis New Issue
2017-07-11 06:52 Michalis Kamburelis File Added: freeing_item_on_set.lpr
2017-07-11 09:34 Maciej Izak Assigned To => Maciej Izak
2017-07-11 09:34 Maciej Izak Status new => assigned
2017-07-11 09:37 Michael Van Canneyt Fixed in Revision => 36724
2017-07-11 09:37 Michael Van Canneyt Note Added: 0101660
2017-07-11 09:37 Michael Van Canneyt Status assigned => resolved
2017-07-11 09:37 Michael Van Canneyt Fixed in Version => 3.2.0
2017-07-11 09:37 Michael Van Canneyt Resolution open => fixed
2017-07-11 09:37 Michael Van Canneyt Assigned To Maciej Izak => Michael Van Canneyt
2017-07-11 09:37 Michael Van Canneyt Target Version => 3.1.1
2017-07-11 09:38 Michael Van Canneyt Note Added: 0101661