Olevariant and memory leak

[NamoRamana 3]

We have an old windows service, recently upgraded in Delphi Rio, that spawns off few threads. Each thread is interacting with COM DLLs. Each thread is working on its own and doesn't need synchronization. Each thread works on a separate record, so no issues of locking. When I run following code, the memory rapidly increases in the Task Manager as threads process records. Its also reported as BSTR (Widestring) type leak in Deleaker. And finally, we end up having the "out of memory".

Each thread does following:

    var
        a: IBusinessClass;
    begin
        a := ObjCOMServer.GetNewInstance('TBusinessClass') // Not the actual call, but the idea is to ask a COM dll to give a new instance of TBusinessClass
        a.RowId := GetNextId;
        a.AWideStringProperty := 'Test';
        a.AnIntProperty := 1001;
        ....etc...
        
        SaveCOMServer.Save(a.asXML, ...) // This is the prior to upgrade code. If I comment out this line, leak doesn't happen
        // SaveCOMServer.Save(String(a.asXML), ...) <--- If I execute this statement then it doesn't leak.
    end;

We tried following but still memory increased in the same way above...
(1). Declare a local variable of type OleVariant to hold a.asXML..and pass it to Save().
(2). In the Save() on SaveCOMServer, the first statement we put is "exit" to rule out any SetAsXML() (code below) leaks.

 

Following is not showing any significant increase of memory in Task Manager (so technically, I don't know if it stopped memory leaks)

 

(1). If I don't call Save() et all, then memory doesn't increase. But calling Save() is essential.
(2). Declare a local string variable and assign a.asXML to it and pass it to Save(), then the memory increase stops.
(3). When I apply casting string(a.asAXML) while calling Save(), the memory increase stops. 

 

Some Details:

asXML - is a read and write property of type OleVariant. Its get method - GetAsXML(), uses lib2XML parser and RTTI (old style, TypInfo),  builds up a XML structure of the object and return an OleVariant.

 

    function xxx.GetAsXML: OleVariant;
    var
      lDocument : xmlDocPtr;
      lRootNode: xmlNodePtr;
      lBuffer : xmlCharPtr; //PAnsiChar in LibXML2
      lBufferSize : integer;
      lResult : string;
      lName : string;
    begin
      lDocument := xmlNewDoc(nil);
      try

        // ... Builds up the XMLDocuments here using RTTI
        
        xmlDocDumpMemory(lDocument, @lBuffer, @lBufferSize);
        try
          lResult := lBuffer; // Ansi converted in unicode string
          Result := lResult; // unicode string to olestr
        finally
          xmlFree(lBuffer);
        end;
      finally
        xmlFreeDoc(lDocument);
      end;
    end;
        
  procedure xxx.SetAsXML(const AInputVariant: OleVariant)
  var 
    ...
  begin  
      lOutputStream := TMemoryStream.Create;
      lOutputStream.Position := 0;

      if (VarType(AInputVariant) = varOleStr) then
      begin
        lStringStream := TStringStream.Create(String(AInputVariant)); // <--- Notice the casting
        try
          lStringStream.Position := 0;
          lOutputStream.CopyFrom(lStringStream, lStringStream.Size);
        finally
          lStringStream.Free;
        end;
        
        // Now process the lOutputStream using libXML2 and RTTI and assign back the properties.
      end
   end;    

Save() -- Takes input OleVariant, fills a local blank object with the data from OleVariant as setting local object's asXML := ABusinessObj
Here is how Save() is defined in type library:

      interface Ixxx: Iyyy
      {
        [id(0x00000001)]
        HRESULT _stdcall Save([in] VARIANT ABusinessObj, ......)

Can someone help to figure out what's going on? Why its leaking the memory, and not increasing when String() is applied, and what could be the clean solution?
    
 

Edited September 25, 2020 by NamoRamana

[Anders Melander 1815]

I would examine the call to Save in the CPU view in the debugger to determine when, and if, the passed variant is cleared. I remember there was some leakage issues surrounding variants passed as value parameters to dispatch interfaces but I can't remember the details anymore.

 

Not related to your leak but:

1. Instead of

   String(AInputVariant)

   you could do:

   VarToStr(AInputVariant)

    

2. Instead of copying the TStringStream to a TMemoryStream, why don't you just operate directly on the TStringStream?

3. lStringStream.Position := 0;
   lOutputStream.CopyFrom(lStringStream, lStringStream.Size);

   is the same as

   lOutputStream.CopyFrom(lStringStream, 0);

    

[NamoRamana 3]

Thank you, Anders for your reply. Yes, the OleVariant parameter is passed as Value. Below snippet is from _TLB.pas.

  Ixxx = interface(Iyyy)
    ['{C2D9C6D7-2282-4DF2-B881-D097CA4B46EF}']
    procedure Save(ABusinessObj: OleVariant;.....); safecall;
...

We are planning to experiment with few options, including passing Olevariant parameter by reference. Also planning to experiment with the solution found here: https://stackoverflow.com/questions/3639113/how-do-i-stop-this-variant-memory-leak

var
	tmp: OleVariant;
begin
...
	tmp := a.asXML;
 	SaveCOMServer.Save(tmp...);
	tmp := Unassigned;
...

Your other suggestions are good, too. Thank you for that. To answer your (3), the version I gave out is a stripped out version. AsSaveXML(), the procedure called when you write to the property AsXML(), calls internally few other functions and procedures. The "lOutputStream" is the common stream upon further processing happens to set the properties on an object. Apart from varOleStr, it also checks the varArray (compressed and uncompressed) and build "lOutputStream", ex:

 

  if (VarType(AInputVariant) = varOleStr) then
  begin
	...      
	lOutputStream.CopyFrom(lStringStream, lStringStream.Size);
	....
  end
  else
  begin
    if VarIsArray(AInputVariant) then
    begin
		...
		lOutputStream.WriteBuffer(Data^, Size);
		...
	end;

	// lOutputStream is now ready to further parsing.

 

[NamoRamana 3]

Ok.. I tried passing the OleVariant by ref. That didn't work. I also tried assigning a temp variable "UnAssigned". That didn't work either. 

 

Now I found the following QC: https://quality.embarcadero.com/browse/RSP-14749.

 

This mentions the almost similar situation I have. I tried to look around in System.Win.ComObj to find  DispatchInvoke(). In it, it has try..finally, which calls FinalizeDispatchInvokeArgs() which does the clean up. I think, FinalizeDispatchInvokeArgs() has the same code mention in comment in QC: https://quality.embarcadero.com/browse/RSP-9819

 

Can a Delphi Guru enlighten me with simplicity here -- whether the leak mentioned above is caused by these delphi units? Coz code in System.Win.ComObj and System.Variants are too hot for me... 🔥

Edited September 29, 2020 by NamoRamana

[Anders Melander 1815]

As far as I can see the fix suggested in RSP-9819 has already been implemented in DispatchInvoke (I checked D10.3).

The RSP-14749 issue looks more promising. I'm not sure why it was closed with the resolution "test case error" but since it's marked as fixed in 10.4.1 I compared the source of ComObj.pas and the only change in it is this (10.3.1 on the left, 10.4.1 on the right):

That does actually look like a fix for the issue in RSP-14749.

If you don't have 10.4.1 you can try patching the 10.3 ComObj.pas with the above change to see if solves the problem for you.

[Dave Nottage 563]

51 minutes ago, Anders Melander said:

That does actually look like a fix for the issue in RSP-14749.

Looks more like a fix for: https://quality.embarcadero.com/browse/RSP-23095

 

Two birds with one stone (perhaps unintentionally)?

[pyscripter 694]

This one was fixed in 10.4  https://quality.embarcadero.com/browse/RSP-23093

@NamoRamanaYou should provide a simple test case that reproduces the issue.  Otherwise we are all guessing and hand-waving.

[NamoRamana 3]

Ok, Guys.. took me a while to come up with reproducible code.. But its attached here. You will see that with each click on a button, the memory increases. We also ran the same code under D 10.4.1 and unfortunately, the behavior is the same -- it keeps on increasing the memory.

 

COMBug.zip

 

[pyscripter 694]

15 minutes ago, NamoRamana said:

Ok, Guys.. took me a while to come up with reproducible code.. But its attached here. You will see that with each click on a button, the memory increases. We also ran the same code under D 10.4.1 and unfortunately, the behavior is the same -- it keeps on increasing the memory.

 

COMBug.zip

 

function TObjSvr.ObjAddRef: Integer;
begin
  OutputDebugString(PChar(Format('TObjSvr ($%08X) AddRef', [integer(Self)])));
end;

function TObjSvr.ObjRelease: Integer;
begin
  OutputDebugString(PChar(Format('TObjSvr ($%08X) Release', [integer(Self)])));
end;

Υour are overriding ObjAddRef, ObjRelease, without calling the inherited method. and you expect to see no memory leaks?

[NamoRamana 3]

My bad.. Even when you take these 2 functions out and comment out all OutputDebugString() from Initialize and Destroy, it still increases the memory in Rio.. We are currently testing in 10.4.1.. Will post the update about that later.

 

COMBug.zip

Edited October 1, 2020 by NamoRamana

[NamoRamana 3]

Update: Thank you very much, @pyscripter for pointing missing inheritance. We modified the code accordingly and tested in both the versions... In Rio, we still see the  memory increase.. But in 10.4.1, it did not.

 

Thank you Anders and Dave as well.

 

In conclusion, we are planning to upgrade our code base to 10.4.1.

Edited October 1, 2020 by NamoRamana

[pyscripter 694]

1 minute ago, NamoRamana said:

Update: Thank you very much, @pyscripter for pointing missing inheritance. We modified the code accordingly and tested in both the versions... In Rio, we still see the  memory increase.. But in 10.4.1, it did not.

Glad to hear that!  So it must be  https://quality.embarcadero.com/browse/RSP-23093 that was causing the issue.

[Anders Melander 1815]

1 minute ago, pyscripter said:

RSP-23093 

That was the performance issue. RSP-14749 was the leak.

But as Dave suggested, the change in ComObj probably fixed both.

[pyscripter 694]

1 minute ago, Anders Melander said:

That was the performance issue. RSP-14749 was the leak.

No the performance issue was https://quality.embarcadero.com/browse/RSP-23095 also fixed as suggested.

 https://quality.embarcadero.com/browse/RSP-23093 was a memory leak issue.  Please read the reports.

 

In both issues the suggested solution was implemented.

[Anders Melander 1815]

1 minute ago, pyscripter said:

No the performance issue was https://quality.embarcadero.com/browse/RSP-23095 also fixed as suggested.

Yes you're right. That's what I get for thinking I can remember issue numbers while watching a Counter Strike match..