2

Below is the thought of Freeing Objects in Delphi's TStrings Items by Zarko Gajic at at About.com.delphi I am using Delphi 7, TStringList does not have OwnsObjects.

Running the following code will prompt EaccessViolation error. I donot know why and how to walk around it to free objects.

Thanks a lot.

procedure TForm6.freelist(const alist: TStringList);
var
  i: integer;
begin
  try
    for i:=0 to pred(alist.Count) do begin
      if Assigned(alist.Objects[i]) then begin
        alist.Objects[i].Free;       
        alist.objects[i]:=nil;       
      end;                           
    end;
    alist.Clear;
  finally
    alist.Free;
  end;
end;

Edit:

I add this line, alist.Objects[i]:=Pointer(0); and there is no error.

...

for i:=0 to pred(alist.Count) do begin
          if Assigned(alist.Objects[i]) then begin
            alist.Objects[i]:=Pointer(0);  //newly added line. 
            alist.Objects[i].Free;       
            alist.objects[i]:=nil;       
          end;                           
        end;
...

//But I do not know if this is correct and 
// if the efficiency will be compromised.
//This is awkward method?
Warren
  • 795
  • 1
  • 10
  • 19
  • 2
    The problem is in the code the populates the list. Anyway, I believe TStringList is not what you need. you need TObjectList with OwnsObjects set to true. Move the string into your object. – David Heffernan Feb 05 '12 at 11:03
  • @DavidHeffernan Thanks a million. It is an alternative way to use TobjectList. Currently, Freeing objects in TStringlist is an impossible task? – Warren Feb 05 '12 at 11:10
  • I agree with @DavidHeffernan. The code above isn't the problem. If you are getting Access Violation it is because you put something in the Object property of that StringList's Item that isn't nil (thus assigned is true) and doesn't point to a valid object instance. – Marjan Venema Feb 05 '12 at 11:26
  • I'm sure that @DavidHeffernan is correct. I have used similar code many times without any problems. If the objects array contains only valid object references, you should be able to free them OK. – Martin James Feb 05 '12 at 11:30
  • 1
    freeing objects in stringlist works but is more hassle that getting object list to do it. – David Heffernan Feb 05 '12 at 11:55
  • @DavidHeffernan Thanks again for your advice. Please look at the newly added line. It is not an elegant way and I do not know if this is correct, now it has no error prompt. If the stringlist is huge, efficiency will be a problem? – Warren Feb 05 '12 at 12:12
  • @Warren Now it's not doing anything. You're calling `Free` on `Pointer(0)`. Your objects are not being freed at all. As mentioned before, there's nothing wrong with the original code. – Nick Barnes Feb 05 '12 at 12:29
  • Show us how you add the object to the list. based on your previous question, are you adding a New(Record) or a `TClass`? if it's a record you will need to use `Dispose` to releases memory allocated by `New`. – kobik Feb 05 '12 at 12:34
  • 1
    @kobik I am not using New function. I'm just adding object like alist.addobject('john',pointer(a number like 1, or 5). – Warren Feb 05 '12 at 12:40
  • @ NickBarnes you mean in alist.objects[i]:=pointer(0), and free objects[i], the very object[i] is not freed? – Warren Feb 05 '12 at 12:48
  • Back to basics warren. Read this: http://stackoverflow.com/questions/8548843/why-should-i-not-use-if-assigned-before-using-or-freeing-things/8550628#8550628 – David Heffernan Feb 05 '12 at 12:51

2 Answers2

5

The original answer below answers the question you asked. However, it transpires in the comments that you are not adding objects to your string list. You are simply adding integers cast to Pointer. In that case you must not call Free on those integers and that would completely explain your error. So the entire code in your question is gratuitous and you should simply call Free on the list when you are done with it.

When you cast an integer to a pointer and add it to the list, there is no memory allocated beyond that used to store the pointer. That is because an integer is a value type. When you add a true pointer or an object then you are adding a reference type and disposal of that object involves calling Free for an object or FreeMem (or Dispose) for a pointer.

Original answer to the question as asked

Your original code is correct, albeit a little clunky. The problem you have is in the code that populates Objects[]. Since we cannot see that code we can't say what you have got wrong.

Now, having said your code was clunky, here's how I would write it:

procedure ClearList(List: TStringList);
var
  i: Integer;
begin
  for i := 0 to pred(List.Count) do
    List.Objects[i].Free;       
  List.Clear;
end;

Some notes on the above:

  • You do not need the if Assigned(obj) test before calling obj.Free. I explain why not here: Why should I not use "if Assigned()" before using or freeing things?
  • There's little point in setting the items to nil if you are about to call Clear.
  • You should not call List.Free in such a routine. Your list's life time should be managed separately from the code that clears the list. The call to List.Free should be made in the same scope as the call that constructs the list. Or, if this list is a field of a class, then the call to List.Free should be made from the destructor of the owning class.

Now, as I have already said in comments to this question and your previous question, all this lifetime management work with the Objects[] property of a Delphi 7 string list is very unsatisfactory. It's all too easy to leak objects. I would recommend the following alternatives:

  1. Switch to using TObjectList rather than TStringList. Set the OwnsObjects property to True when you create the list. This will ensure that when an item is deleted from the list, it will be destroyed at the point of removal. You simply hand to the list the responsibility for lifetime management of its items. Note that this will require you to move the string that you are storing in your current string list code to be a property of the object. But that is invariably the correct approach anyway so I see that as an upside rather than a drawback.
  2. If you do wish to keep using a string list, create your own derived class that handles ownership, probably by adding a property named OwnsObjects. Push this problem onto the list and let your higher level code be free from that concern. Debug the code once in the context of the list class and then re-use it over and over again safe and secure in the knowledge that it works.
Community
  • 1
  • 1
David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
  • 1
    Might want to rethink your TObjectList recommendation in light of his last couple of comments above... His `Objects[]` isn't full of objects. It's full if integers cast to pointers. – Nick Barnes Feb 05 '12 at 13:47
  • @Nick Thanks I had not seen that. I was merely answering the question that was asked. – David Heffernan Feb 05 '12 at 14:11
  • @DavidHeffernan Yes, your answer hits home. Thanks for so clear and detailed explanation. – Warren Feb 05 '12 at 14:27
-1

Just a tip: Maybe the use of TStringList is just to use .indexOf(MyString) to look for an item.

In such case, moving code to TObjectList is a total madness, you must re-invent the wheel, better said, the look on the list.

My recommendation is that it is better to avoid at all setting an object pointer to what is not an object, it is very easy to put a cat from Integer to Pointer, but it is also better to create an TIntegerObject that holds such integer (not in terms of speed and memory, just in terms of not breaking the definitions).

I must explain such: If you store an Integer (not an object) where an Object is supposed to be, you are prone in a future to get pointer error operations when by error you forgot that pointer does not point to anything.

Oh, and there is another thing you loose by just casting: Knowing it it has a valid value or not; that is because Integer(Nil) equals value zero, so you can not know if you have a Nil or a zero value cast as an object pointer.

If you want it very simplified, aka without property, get ans set methods, etc, here is a sample:

TIntegerObject=class(TObject)
public
  value:Integer;
  constructor Create(value:Integer);
end;

TIntegerObject.Create(value:Integer);
begin
     inherited Create;
     Self.value:=value;
end;

It is a very basic class to store an integer as an object, there is no need for it to have a destructor since inside it there is nothing created that need to be free.

Then when you want to store an Integer onto the Objects of a TStringList you simply do:

MyStringList.Objects[MyIndex]:=TIntegerObject.Create(MyIntegerValue);

You can get the index for a known string value just with:

MyIndex:=MyStringList.IndexOf(MyString);

Then check it it returns -1, that means such string is not on the list, it it is greater it means the string is on that position of the list.

Using this approach you can check if you have an Integer on any element just comparing it to Nil, like with:

if Nil=MyStringList.Objects[MyIndex]
then begin
     end
else begin
     end;

And to get the integer value is as simple as:

MyInteger:=TIntegerObject(MyStringList.Objects[MyIndex]).value;

Hope this helps people to better code.

I know it is much easier to code it as just MyStringList.Objects[MyIndex]:=TObject(MyInteger); and MyInteger:=Integer(MyStringList.Objects[MyIndex]);, but that is like doing a FAKE/LIE, doing such you are telling MyInteger has the memory start position of an object that does not really exists; that is prone to cause a lot of errors when used as what it is, a memory pointer, such memory address may not belong to the app, such can cause errors or also something worst, code injection security holes, etc., by the way, that is very well common known to had been used to crack consoles securities, jailbreacks, etc.

Of course creating an object for each Integer is a waste of time and memory.

It is up to the coder, to use one or another (or other) approach, but remember to be very precise (now and on the future) and my recommendation is if you cast Integers as Objects (or pointers) add a comment and never ever try to free them.

Another warning: setting some properties of some objects to Nil can cause a Free, and if such 'pointer' points to a what is not an object or a memory allocated by the app, will cause an error.

Just an example of the last thing, setting a TObjectList (with OwnsObjects property set to True) will do a Free on each element as well as a Free on the list it self (at least on what i have tested) so doing any of this lines of code Frees the objects and the list:

Remember that MyObjectList for this sample has OwnsObjects property set to True.

MyObjectList:=Nil; // This will actually free the whole list and Nil it;
MyObjectList.Free; // This will actually free the whole list but not Nil it;

You must decide what to use, I just prefer to be on the safe line, I can not know how many years in the future I will be on the need to edit the code.

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
Laura
  • 1
  • -1. `MyObjectList:=Nil;` will not free the object pointed to by the `MyObjectList` variable. It will only set the variable to point to `nil` instead of the object. The object will remain in memory. – Andreas Rejbrand Nov 03 '18 at 10:07