Link to home
Start Free TrialLog in
Avatar of BdLm
BdLmFlag for Germany

asked on

try to call free objectList

I have a Class :    TPointList = class(TList) .......end;  to store drawing points. In the code fragment below you  you can see my last lines of code, where I would like to free the memory again.
This list contains eg. 51 Elements at the end of the process, but I can't free the second PointList, error is AV  while calling free mem. ; Why is the list full with 51 Elements but I Can't free iT???
Any good idea to get more information on the free failure, already using madshi and fastMM4

-----------   free  point list  ---------------
Your SV List contains 51 Points
Your GL List contains 51 Points
Outmemo.lines.add('-----------   free  point list  ---------------');

      if (SVmatchPoints <> nil) then
                   Outmemo.lines.add('Your SV List contains ' + IntToStr(SVmatchPoints.count) + ' Points' )
                   else
                   Outmemo.lines.add ('Pointist is nil ');


      SVMatchPoints.Free;



      if (GLmatchPoints<> nil)then
                   Outmemo.lines.add('Your GL List contains ' + IntToStr(GLmatchPoints.count) + ' Points')
                   else
                   Outmemo.lines.add ('Pointist is nil ');


      GLMatchPoints.Free;

Open in new window

free-pointlist-error.png
Avatar of MerijnB
MerijnB
Flag of Netherlands image

Can you give details of the AV please?
Avatar of BdLm

ASKER

invalid debugger exception , see screen dump of the failure report

invalid-ee.jpg
SOLUTION
Avatar of MerijnB
MerijnB
Flag of Netherlands image

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
we would need to know what is the definition & implementation of TPointList = class(TList)
Especially constructor, destructor, and maybe all calls that add elements to these list.
Avatar of BdLm

ASKER

OK, here come the implementation part of TPointList :


the items i stote in that list :

     PPointItem= ^TPointItem;                                { Dyn. Liste mit Punkten          }
     TPointItem= record                                           { Pixel und RealWorld Koordinaten }
                 Index      : Integer;
                 PixelPoint : TPoint;
                 WorldPoint : FPoint;
                 PointStr   : String;
                 end;


  ///  a class for storing point values ...
  TPointList = class(TList)
   private

    function GetItem(AIndex: Integer): PPointItem;



  public
    constructor create;

    destructor Destroy; override;

    function   Add(aItem: PPointItem) : integer;

    function   GetItemByName(AName: String): PPointItem;

    function   CalcGP :  FPoint;

    property   gp  :  FPoint  read CalcGP;
  end;

constructor TPointList.create;
begin
    inherited Create;
end;
function TPointList.Add(aItem: PPointItem): integer;
begin
     result := inherited Add(aItem);
end;
destructor TPointList.Destroy;
begin
  try
     Clear;
  Finally
    inherited Destroy;
  end;
end;

Open in new window

your TPointList create/destroy is useless, they are doing what TList do already :

destructor TList.Destroy;
begin
  Clear;
end;

what you are not doing on the other hand, is overriding Clear to actually free your records (if they are not stored elsewhere, which is doubtful). And you would have also to override Delete.
That is always tricky to use TList because of that. You should use TObjectList instead, with your record not a record but an object (very simple objects are like record pointers but much easier to manage)

TObjectList will destroy all objects that are removed, deleted or cleared (3 names for almost the same thing) from the list. Nothing more to do than this :
TPointItem= class(TObject)
 public
  Index      : Integer;
  PixelPoint : TPoint;
  WorldPoint : FPoint;
  PointStr   : String;
 end;

TPointList = class(TObjectList)
 private
  function GetItem(AIndex: Integer): TPointItem;
  function CalcGP :  FPoint;
 public
  function GetItemByName(AName: String): TPointItem;

  property GP: FPoint  read CalcGP;
 end;


function TPointList.GetItem(AIndex: Integer): TPointItem;
begin
 Result:=TPointItem(Items[AIndex]);
end;

function TPointList.GetItemByName(AName: String): TPointItem;
Var i:integer;
begin
 for i:=0 to Count-1 do 
  begin
   Result:=GetItem(i);
   if Result.PointStr=aName Then Exit;
  end;
 Result:=nil; // not found
end;

Open in new window

ah, I forgot the creation / Add part :

Var
 aPoint:TPointItem;
begin
 aPoint:TPointItem.Create;
 aPoint.Index:=1; // do you need that ? the point have an index in the TPointList already, to know it from the object  : PointList.IndexOf(aPoint);
 aPoint.PointStr:='My First Point');
 PointList.Add(aPoint);
end;

Open in new window

Avatar of BdLm

ASKER

need to find the problem  using TList, this class is used in a lot of other code,
no chance all code to TObjectList
 
Avatar of BdLm

ASKER

BTW:using delphi debug dcu I end up here

ee-debug-dcu.jpg
> need to find the problem  using TList, this class is used in a lot of other code,
> no chance all code to TObjectList

TObjectList is a descendant of TList. The way it's going to be used won't change much.
When it does, it's to do things on your behalf, so that is a simplification. Your error is an obvious proof that you don't manage the memory correctly : You try to destroy the same pointer twice (or worse).

You have to understand that your problem is not so much in your list (except that you did not override Clear to free the pointers) but in the way you use them. Using TList and manually get/free memory is too much cause for problems. Objects and object lists are much easier to use.

Whatever the way you want to choose, you have to check all your code for creation/destruction of pointers.
Instead of spending month (if your application is so huge) to pinpoint what you did wrong, which we cannot tell you here, you'd better switch your List to TObjectList and your record to TObject, because most of the freeing will simply disappear, all managed by the ObjectList when it is cleared or when you delete items from the list.
Avatar of BdLm

ASKER

I totally aggree with your comments,

is there a chance

a) to override the destroy , free methode and show a message Box

b) to show the varriable name in the message box

the code in the fail case is not that long, I did not call *.free  or *.delete.
In another project using tPointList things are different, but that not discussed here.

destructor TPointList.Destroy;
begin
  try
    application.messagebox('error','you called destroy', mkOK);
  Finally
    inherited Destroy;
  end;
end;

Open in new window

Avatar of BdLm

ASKER

this idea does not work


  aPtList := TPointList. Create;
  or
  aPtList := TPointList. Create('aPtList');


constructor TPointList.create; override; overload;
begin
    inherited Create;
end;

constructor TPointList.create(AInstanceName: String); override;
begin
      FInstanceName :=AInstanceName;
end;

Open in new window

a) that would not be very useful to show a message whenever the list is destroyed. It should not be destroyed very often, so you might as well put a breakpoint.
But even that is not useful, it's the object destruction that you must keep track of. It's not possible with records. With objects, yes it's possible to override Destroy to do things when one is destroyed, be it by a manual call of Free in your code, or of ObjList.Delete(IndexOfObject) or Clear
I don't recommend using messageBox, you'll have too much of those. Start with a memo log of creation/destruction of your objects.
Still, before you do that, you will have to switch to Object/TObjectList , and in the process have a look again of your creation/destructions
That alone could fix things

b) unfortunately, no that is not possible. The variable name is only known by the compiler, and even then not at this point. A same object could be referenced by many variables, or referenced within a list (which will be your case). What you can do, is to add a counter that you increment each time you create one object, put it in a field of this object, and you can show it in your logs when you do things on your objects (mostly destroy them). That is the best you can do.
ok, again that is not the list that you must track but the objects. Never mind here is what you are trying to do :
/* THAT IS UNNECESSARY : You are only calling the inherited
constructor TPointList.create; override; overload;
begin
    inherited Create;
end;
*/

constructor TPointList.create(AInstanceName: String); // NO override, this signature does not exist in the ancestor 
begin
 FInstanceName :=AInstanceName;
 inherited Create; // You still must call the inherited (or another constructor calling it)
end;

Open in new window

Avatar of BdLm

ASKER

why I would like to overload the class constructor

aPointList := TPointList.create;  //  used in many line of codes  in other projects

aPointList := TPointList.create ('aPointList'); // used only in my failing application 

Open in new window

Avatar of BdLm

ASKER

could lokalize the error,

I'm using a function
copyPointListtoDynamicArray(aPtList, bptList, var dynmatrix : array of real)
the size of the matrix is defined inside the function with setlength.
If I call this function directly no problem, but I used to place this function inside a dll,
therefore the problem happend.

Should I open a second threat for further discussion ???

> why I would like to overload the class constructor
YOU DON'T HAVE TO REDECLARE  A METHOD, CONSTRUCTOR OR DESTRUCTOR THAT ALREADY EXIST IN YOUR ANCESTOR IF YOU DON'T CHANGE ANYTHING !
So you cant forget this
/* THAT IS UNNECESSARY : You are only calling the inherited
constructor TPointList.create; override; overload;
begin
    inherited Create;
end;
*/

and still call this
TPointList.create

or declare this if you prefer :
constructor TPointList.create(AInstanceName: String=''); // NO override, this signature does not exist in the ancestor
begin
 FInstanceName :=AInstanceName;
 inherited Create; // You still must call the inherited (or another constructor calling it)
end;

which will be called when using TPointList.create , with default parameter AInstanceName = ''
Avatar of BdLm

ASKER

@:epasquier:
did you see my last post , now the dll issue will be the correct solution,
may be i can not create a dynamic array inside a dll ???

should I close this question and open a new one ?

Avatar of BdLm

ASKER

pls. wait for my next post, may be I solved the issue now
if both your dll and application are made with Delphi, there should be no problem IF you include ShareMem (for dynamic allocation across dll). but it's slow , so you might want to give a try with Fast ShareMem that you can find here :
http://www.torry.net/pages.php?id=228
Avatar of BdLm

ASKER

what I did in the failing function

loop all points in the list :

  aPoint := APointList.Items[i]

 aMatrix[i,j]   := aPoint.PixelPoint.x;  

what solved the problem :


  new(APoint);

  aPoint := APointList.Items[i]
 
  aMatrix[i,j]   := aPoint ^.PixelPoint.x;


as I undersand my error: the failing code copy the address of a PointList to the matrix, with the pass code the values is copied to the matrix ...  
 
ASKER CERTIFIED SOLUTION
Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
Avatar of BdLm

ASKER

the experts did a good job, even the solution has been on a different part of my code, helped to find the problem