Forum > LCL

[SOLVED] TXMLConfig bug was already reported.

(1/2) > >>

omurolmez:
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  [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---procedure TXMLConfig.LoadFromFile(const AFileName: string); Var  F : TFileStream; begin  F:=TFileStream.Create(AFileName,fmOpenread or fmShareDenyWrite);  try    FFileName := '';    ReadXMLFile(Doc, AFilename);    FFileName:=AFileName;  finally    F.Free;  end;end; 
I think it calls ReadXMLFile of this overloaded version (xmlread.pp line 410) :

--- Code: ---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;
--- End code ---

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: ---var
  FXMLConfig :TXMLConfig;
begin
  FXMLConfig:= TXMLConfig.Create(nil);
  FXMLConfig.LoadFromFile(FConfigFileName);
  FXMLConfig.Free;
end;
--- End code ---

To achieve issue, I use this below :

--- Code: ---var
  FXMLConfig :TXMLConfig;
begin
  FXMLConfig:= TXMLConfig.Create(nil);
  FXMLConfig.ReadOnly:= true;
  FXMLConfig.Filename:= FConfigFileName;
  FXMLConfig.Free;
end;
--- End code ---

Probably, this may also works below:

--- Code: ---var
  FXMLConfig :TXMLConfig;
begin
  FXMLConfig:= TXMLConfig.Create(nil);
  FXMLConfig.Filename:= '';
  FXMLConfig.FileName:= FConfigFileName;
  FXMLConfig.Free;
end;
--- End code ---

Ömür Ölmez.

TRon:

--- Quote from: omurolmez 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

--- End quote ---
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  %)

omurolmez:
Thank you for your interest. I corrected my post.

Do you know, how I can report these issues ?

Regards
Ömür Ölmez.

TRon:
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:
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  [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---procedure TXMLConfig.LoadFromFile(const AFileName: string);Var  F : TFileStream;begin  F:=TFileStream.Create(AFileName,fmOpenread or fmShareDenyWrite);  try    FFileName := '';    LoadFromStream(F);    FFileName:=AFileName;  finally    F.Free;  end;end;     

Navigation

[0] Message Index

[#] Next page