View Issue Details

IDProjectCategoryView StatusLast Update
0033516FPCRTLpublic2018-04-07 21:02
ReporterFiliuta VitaliAssigned ToMichael Van Canneyt 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionno change required 
Product Version3.1.1Product Build 
Target Version3.2.0Fixed in Version3.1.1 
Summary0033516: Fix for TCollection.Destroy
Descriptiondestructor TCollection.Destroy;
begin
  FUpdateCount:=1; // Prevent OnChange
  try
    DoClear;
  Finally
    FUpdateCount:=0;
  end;
  FItems.Free;
  Inherited Destroy;
end;

You should not call DoClear, without checking, that FItems <> nil. If constructor of descendant class fails (it might happen), destructor will be called, and FItems could be nil here, so we will get an EAccessViolation, that will hide more informative exception (from constructor).

Patch is attached.
TagsNo tags attached.
Fixed in Revision38627
FPCOldBugId
FPCTarget
Attached Files
  • collect.Destroy.diff (591 bytes)
    Index: rtl/objpas/classes/collect.inc
    ===================================================================
    --- rtl/objpas/classes/collect.inc	(revision 38588)
    +++ rtl/objpas/classes/collect.inc	(working copy)
    @@ -302,13 +302,16 @@
     
     destructor TCollection.Destroy;
     begin
    -  FUpdateCount:=1; // Prevent OnChange
    -  try
    -    DoClear;
    -  Finally
    -    FUpdateCount:=0;
    +  if FItems <> nil then
    +  begin
    +    FUpdateCount:=1; // Prevent OnChange
    +    try
    +      DoClear;
    +    Finally
    +      FUpdateCount:=0;
    +    end;
    +    FItems.Free;
       end;
    -  FItems.Free;
       Inherited Destroy;
     end;
     
    
    collect.Destroy.diff (591 bytes)

Activities

Filiuta Vitali

2018-03-26 18:24

reporter  

collect.Destroy.diff (591 bytes)
Index: rtl/objpas/classes/collect.inc
===================================================================
--- rtl/objpas/classes/collect.inc	(revision 38588)
+++ rtl/objpas/classes/collect.inc	(working copy)
@@ -302,13 +302,16 @@
 
 destructor TCollection.Destroy;
 begin
-  FUpdateCount:=1; // Prevent OnChange
-  try
-    DoClear;
-  Finally
-    FUpdateCount:=0;
+  if FItems <> nil then
+  begin
+    FUpdateCount:=1; // Prevent OnChange
+    try
+      DoClear;
+    Finally
+      FUpdateCount:=0;
+    end;
+    FItems.Free;
   end;
-  FItems.Free;
   Inherited Destroy;
 end;
 
collect.Destroy.diff (591 bytes)

Michael Van Canneyt

2018-03-26 18:29

administrator   ~0107426

Free already checks for Nil, no change is required.

see
https://www.freepascal.org/docs-html/current/rtl/system/tobject.free.html

Michael Van Canneyt

2018-03-26 18:32

administrator   ~0107427

My apologies, I misread the comment. I added a check for DoClear.

Issue History

Date Modified Username Field Change
2018-03-26 18:24 Filiuta Vitali New Issue
2018-03-26 18:24 Filiuta Vitali File Added: collect.Destroy.diff
2018-03-26 18:29 Michael Van Canneyt Note Added: 0107426
2018-03-26 18:29 Michael Van Canneyt Status new => resolved
2018-03-26 18:29 Michael Van Canneyt Resolution open => no change required
2018-03-26 18:29 Michael Van Canneyt Assigned To => Michael Van Canneyt
2018-03-26 18:30 Michael Van Canneyt Status resolved => assigned
2018-03-26 18:32 Michael Van Canneyt Fixed in Revision => 38627
2018-03-26 18:32 Michael Van Canneyt Note Added: 0107427
2018-03-26 18:32 Michael Van Canneyt Status assigned => resolved
2018-03-26 18:32 Michael Van Canneyt Fixed in Version => 3.1.1
2018-03-26 18:32 Michael Van Canneyt Target Version => 3.2.0
2018-04-07 21:02 Filiuta Vitali Status resolved => closed