Recent

Author Topic: [SOLVED] TXMLConfig bug was already reported.  (Read 1046 times)

omurolmez

  • New Member
  • *
  • Posts: 10
[SOLVED] TXMLConfig bug was already reported.
« on: May 08, 2023, 10:47:53 pm »
I think this procedure does not use created TFileStream instance obviously

lazarus\fpc\3.2.2\source\packages\fcl-xml\src\xmlconf.pp line 161

Code: Pascal  [Select][+][-]
  1. procedure TXMLConfig.LoadFromFile(const AFileName: string);
  2.  
  3. Var
  4.   F : TFileStream;
  5.  
  6. begin
  7.   F:=TFileStream.Create(AFileName,fmOpenread or fmShareDenyWrite);
  8.   try
  9.     FFileName := '';
  10.     ReadXMLFile(Doc, AFilename);
  11.     FFileName:=AFileName;
  12.   finally
  13.     F.Free;
  14.   end;
  15. end;
  16.  

I think it calls ReadXMLFile of this overloaded version (xmlread.pp line 410) :
Code: [Select]
procedure ReadXMLFile(out ADoc: TXMLDocument; const AFilename: String);
var
  FileStream: TStream;
begin
  ADoc := nil;
  FileStream := TFileStream.Create(AFilename, fmOpenRead+fmShareDenyWrite);
  try
    ReadXMLFile(ADoc, FileStream, FilenameToURI(AFilename));
  finally
    FileStream.Free;
  end;
end;

procedure ReadXMLFile(out ADoc: TXMLDocument; f: TStream; const ABaseURI: String);
var
  Reader: TXMLTextReader;
  ldr: TLoader;
begin
  ADoc := TXMLDocument.Create;
  Reader := TXMLTextReader.Create(f, ABaseURI, ADoc.Names);
  try
    ldr.ProcessXML(ADoc, Reader);
  finally
    Reader.Free;
  end;
end;

I think, it causes memory leak since out ADoc parameter overwrites old value of TXMLConfig's Doc.

Is FreeAndNil(Doc) call missing before first ReadXMLFile call (like in TXMLConfig.DoSetFileName) ?

To reproduce, compile with heaptrc this below :
Code: [Select]
var
  FXMLConfig :TXMLConfig;
begin
  FXMLConfig:= TXMLConfig.Create(nil);
  FXMLConfig.LoadFromFile(FConfigFileName);
  FXMLConfig.Free;
end;

To achieve issue, I use this below :
Code: [Select]
var
  FXMLConfig :TXMLConfig;
begin
  FXMLConfig:= TXMLConfig.Create(nil);
  FXMLConfig.ReadOnly:= true;
  FXMLConfig.Filename:= FConfigFileName;
  FXMLConfig.Free;
end;

Probably, this may also works below:
Code: [Select]
var
  FXMLConfig :TXMLConfig;
begin
  FXMLConfig:= TXMLConfig.Create(nil);
  FXMLConfig.Filename:= '';
  FXMLConfig.FileName:= FConfigFileName;
  FXMLConfig.Free;
end;

Ömür Ölmez.
« Last Edit: May 22, 2023, 06:27:55 pm by omurolmez »

TRon

  • Hero Member
  • *****
  • Posts: 2435
Re: TXMLConfig Possible Bugs (fpc 3.2.2 lazarus 2.2.6)
« Reply #1 on: May 08, 2023, 11:10:06 pm »
I think this procedure does not use created TFileStream instance obviously

lazarus\fpc\3.2.2\source\packages\fcl-xml\src\xmlconf.pp line 161
Indeed. What a strange implementation.

btw; The test snippet that you posted does not free the component which leads to an additional leak

Aside from that, I am able to confirm the leaks (*)

(*) when you place the xmlconf.pp unit in your project directory and do a fresh build the leaks seems to have vanished  %)
« Last Edit: May 08, 2023, 11:18:49 pm by TRon »

omurolmez

  • New Member
  • *
  • Posts: 10
Re: TXMLConfig Possible Bugs (fpc 3.2.2 lazarus 2.2.6)
« Reply #2 on: May 08, 2023, 11:25:06 pm »
Thank you for your interest. I corrected my post.

Do you know, how I can report these issues ?

Regards
Ömür Ölmez.

TRon

  • Hero Member
  • *****
  • Posts: 2435
Re: TXMLConfig Possible Bugs (fpc 3.2.2 lazarus 2.2.6)
« Reply #3 on: May 08, 2023, 11:32:54 pm »
Thank you for the correction.

When you read this text then move your eyes to the left. There is list of items under the 'chapter' that reads "Free Pascal". One of those items is "BugTracker" you can press on that or press on the following link: https://gitlab.com/freepascal.org/fpc/source/-/issues

You do require a gitlab account for that though. If you do not have such an account or have no (further) interest of using one then you could ask someone who reads this and does have a gitlab account to report the error for you.

TRon

  • Hero Member
  • *****
  • Posts: 2435
Re: TXMLConfig Possible Bugs (fpc 3.2.2 lazarus 2.2.6)
« Reply #4 on: May 08, 2023, 11:44:41 pm »
Before reporting the error though, could you conform something for me ?

How did you install the Free Pascal compiler ?

Reason to ask is because the leaks themselves seem related to a prebuild Free Pascal distribution.
The implementation of loadfromfile however seems wrong to me no matter what.

nevermind, found it -> https://gitlab.com/freepascal.org/fpc/source/-/issues/40126 (the leaks have been reported reported (and fixed)).


The loadfromfile function should imho look more like:
Code: Pascal  [Select][+][-]
  1. procedure TXMLConfig.LoadFromFile(const AFileName: string);
  2. Var
  3.   F : TFileStream;
  4. begin
  5.   F:=TFileStream.Create(AFileName,fmOpenread or fmShareDenyWrite);
  6.   try
  7.     FFileName := '';
  8.     LoadFromStream(F);
  9.     FFileName:=AFileName;
  10.   finally
  11.     F.Free;
  12.   end;
  13. end;    
  14.  
« Last Edit: May 09, 2023, 12:13:14 am by TRon »

omurolmez

  • New Member
  • *
  • Posts: 10
Re: TXMLConfig Possible Bugs (fpc 3.2.2 lazarus 2.2.6)
« Reply #5 on: May 09, 2023, 01:35:10 pm »
@TRon thank you for your kind interest.

By the way, for who use LoadFromFile, setting FileName seems flushes and saves old file implicitly, if filename points to a valid exist file before set.
Code: [Select]
var
  FXMLConfig :TXMLConfig;
begin
  FXMLConfig:= TXMLConfig.Create(nil);
  FXMLConfig.Filename:='file1.xml';
  //change data in FXMLConfig
  FXMLConfig.Filename:= ''; //<- it seems flushes and saves 'file1.xml'
  FXMLConfig.FileName:= 'file2.xml';
  //do something
  FXMLConfig.Free;
end;

So, it is better to use this
Code: [Select]
var
  FXMLConfig :TXMLConfig;
begin
  FXMLConfig:= TXMLConfig.Create(nil);
  FXMLConfig.Filename:='file1.xml';
  //change data in FXMLConfig
  FXMLConfig.ReadOnly:= true;
  FXMLConfig.Filename:= 'file2.xml';
  //do something
  FXMLConfig.Free;
end;
« Last Edit: May 09, 2023, 01:40:58 pm by omurolmez »

TRon

  • Hero Member
  • *****
  • Posts: 2435
Re: TXMLConfig Possible Bugs (fpc 3.2.2 lazarus 2.2.6)
« Reply #6 on: May 09, 2023, 11:04:45 pm »
My understanding of the XMLConfig component in a nutshelled example:

Code: Pascal  [Select][+][-]
  1. program xmling;
  2.  
  3. {$mode objfpc}{$H+}
  4.  
  5. uses
  6.   xmlconf;
  7.  
  8. procedure Explain;
  9. var
  10.   FXMLConfig :TXMLConfig;
  11. begin
  12.   FXMLConfig:= TXMLConfig.Create(nil);
  13.   FXMLConfig.Filename:='file1.xml';
  14.   //change data in FXMLConfig
  15.   FXMLConfig.Filename:= '';
  16.   FXMLConfig.FileName:= 'file2.xml';
  17.   //            |
  18.   //            V
  19.   //  Changing the filename internally calls the flush method when the name
  20.   //  differs from the previous one (and when the filename is not an empty one).
  21.   //
  22.   //  Thus it not only "seems" that way as it actually does so :-)
  23.   //
  24.   //do something
  25.   FXMLConfig.Free;
  26. end;
  27.  
  28. procedure ManualSaving;
  29. var
  30.   XMLConfig :TXMLConfig;
  31. begin
  32.   XMLConfig:= TXMLConfig.Create(nil);
  33.   XMLConfig.LoadFromFile('file1.xml');
  34.   //change data in FXMLConfig
  35.   if XMLConfig.Modified and not XMLConfig.ReadOnly // this (more or less) does
  36.     then XMLConfig.SaveToFile('file1.xml');        // the same as flush
  37.   XMLConfig.ReadOnly:= True;
  38.   XMLConfig.LoadFromFile('file2.xml');             // load the new xml
  39.   //do something
  40.   XMLConfig.Free;
  41. end;
  42.  
  43. procedure SaveByUsingFlush;
  44. var
  45.   XMLConfig :TXMLConfig;
  46. begin
  47.   XMLConfig:= TXMLConfig.Create(nil);
  48.   XMLConfig.LoadFromFile('file1.xml');
  49.   //change data in FXMLConfig
  50.   XMLConfig.Flush;       // calling flush saves the current file ('file1.xml')
  51.                          // when modified (when not in readonly mode and the
  52.                          // filename is not empty).
  53.   XMLConfig.ReadOnly := true;
  54.   XMLConfig.LoadFromFile('file2.xml'); // load the new xml
  55.   //do something
  56.   XMLConfig.Free;
  57. end;
  58.  
  59. begin
  60.   Explain;
  61.   ManualSaving;
  62.   SaveByUsingFlush;
  63. end.
  64.  

wp

  • Hero Member
  • *****
  • Posts: 11857
Re: TXMLConfig Possible Bugs (fpc 3.2.2 lazarus 2.2.6)
« Reply #7 on: May 09, 2023, 11:14:06 pm »
You might also consider to have a look at the TXMLConfig in the Lazarus unit "laz2_xmlcfg" which is a fork of the FPC unit xmlconf. It is well-tested because it is used by the IDE to store all its settings in xml files.

From the help file: "laz2_xmlcfg.pas contains classes, type and routines used to read and write configuration data files in XML format. It is copied from the FreePascal Free Component Library (FCL) and adapted to use UTF-8 strings instead of WideStrings. This file is part of the LazUtils package. "
« Last Edit: May 09, 2023, 11:17:29 pm by wp »

TRon

  • Hero Member
  • *****
  • Posts: 2435
Re: TXMLConfig Possible Bugs (fpc 3.2.2 lazarus 2.2.6)
« Reply #8 on: May 10, 2023, 02:30:15 am »
A good advise wp, though not everyone is up for that.

Although the loadfromfile implementation (from xmlconf) does not hurt function-wise it does hurt reading it  :)

omurolmez

  • New Member
  • *
  • Posts: 10
Re: TXMLConfig Possible Bugs (fpc 3.2.2 lazarus 2.2.6)
« Reply #9 on: May 17, 2023, 01:27:48 am »
@wp, @TRon thank you both for your help.

 

TinyPortal © 2005-2018