Recent

Author Topic: xml read memory leak question  (Read 4202 times)

k1attila1

  • Jr. Member
  • **
  • Posts: 99
xml read memory leak question
« on: December 07, 2021, 06:45:04 am »
Code: Pascal  [Select][+][-]
  1. uses Laz2_DOM, laz2_XMLRead;
  2.  
  3. var
  4.     doc:TXMLDocument;
  5.     sz1 : TDOMNode;
  6. begin
  7.   doc:=TXMLDocument.Create;
  8.   ReadXMLFile(doc,'alma.xml');
  9.  
  10.   sz1:=doc.FindNode('szamla');
  11.  
  12.   sz1.Free;                     <-- THE QUESTION : is this line important ?
  13.   doc.Free;                     //i think not because sz1 only a pointer in doc
  14. end;

and what about if : sz1.CreateElement(...   // in this case do we need sz1.free;  ????

thank you Attila

[Edited to add code tags - please read How to Use the Forums.]
« Last Edit: December 07, 2021, 11:29:50 am by trev »

dbannon

  • Hero Member
  • *****
  • Posts: 1815
    • tomboy-ng, a rewrite of the classic Tomboy
Re: xml read memory leak question
« Reply #1 on: December 07, 2021, 07:56:33 am »
Assuming you are using the Lazarus IDE, go to Projects->ProjectOptions->Debugging and click the "Use HeapTrc Unit".

Then, run your app with and without the line you are are interested in and see the difference. On Linux, the output appears on the Console output, on Windows (and Mac I think) pops up on a separate window on application close.  Its quite important you run that test from time to time....

Davo
Lazarus 2, Linux (and reluctantly Win10, OSX)
My Project - https://github.com/tomboy-notes/tomboy-ng

k1attila1

  • Jr. Member
  • **
  • Posts: 99
Re: xml read memory leak question
« Reply #2 on: December 07, 2021, 03:45:46 pm »
thank you, but after this i get 8 unfreed blocks :

 doc:=TXMLDocument.Create;
 ReadXMLFile(doc,'c:\szla1.xml');


 FreeAndNil(doc); 

wp

  • Hero Member
  • *****
  • Posts: 9175
Re: xml read memory leak question
« Reply #3 on: December 07, 2021, 04:54:08 pm »
This would be logical, but the authors of the xml routines decided to go the confusing way. ReadXMLFile reads the xml document, but it also creates it - nothing in the procedure name to indicate that...

So, do not create the XMLDocument explicitely. The following code is enough:

Code: Pascal  [Select][+][-]
  1. uses
  2.   laz2_xmlread, laz2_dom;
  3. var
  4.   doc: TXMLDocument;
  5. begin
  6.   ReadXMLFile(doc, 'project1.lpi');
  7.   doc.Free;
  8. end;

There is no need to free nodes which belong to the doc tree. They all are destroyed by the tree.

Mainly Lazarus trunk / fpc 3.2.0 / all 32-bit on Win-10, but many more...

k1attila1

  • Jr. Member
  • **
  • Posts: 99
Re: xml read memory leak question
« Reply #4 on: December 08, 2021, 06:17:00 am »
thank you

it was the problem.

PascalDragon

  • Hero Member
  • *****
  • Posts: 3664
  • Compiler Developer
Re: xml read memory leak question
« Reply #5 on: December 08, 2021, 09:07:41 am »
This would be logical, but the authors of the xml routines decided to go the confusing way. ReadXMLFile reads the xml document, but it also creates it - nothing in the procedure name to indicate that...

But in the declaration of the routine:

Code: Pascal  [Select][+][-]
  1. procedure ReadXMLFile(out ADoc: TXMLDocument; const AFilename: String); overload;

Pity that the compiler doesn't yet warn that the value assigned to doc isn't used...

wp

  • Hero Member
  • *****
  • Posts: 9175
Re: xml read memory leak question
« Reply #6 on: December 08, 2021, 11:03:13 am »
Code: Pascal  [Select][+][-]
  1. procedure ReadXMLFile(out ADoc: TXMLDocument; const AFilename: String); overload;

Pity that the compiler doesn't yet warn that the value assigned to doc isn't used...
Most users won't understand such a warning (including myself...). It would have been better to write the routine such that the user would be forced to create the doc outside ReadXMLFile. The current version leads to the same mess which Lazarus has with Listbox.Items := FindAllFiles(...). Unfortunately, it cannot be changed any more.
Mainly Lazarus trunk / fpc 3.2.0 / all 32-bit on Win-10, but many more...

 

TinyPortal © 2005-2018