Usage of TInterfacedObject. Automatic release?

classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

Usage of TInterfacedObject. Automatic release?

Luiz Americo Pereira Camara
I'm tracing a memory leak in a Delphi ported component and found that
the culprit was a TInterfacedObject descendant that was not being freed.

I found that if  i directly call Free or _Release the memory leak
vanishes (Delphi code does not call these).

My question is: is TInterfacedObject descendants supposed to be freed
automatically or should i call Free directly?

See attached example.

Using fpc 2.1.4 under win32 XP SP2.

Luiz

program interfaceleak2;

{$mode delphi}

uses
  Heaptrc, Windows, Activex, Classes, SysUtils;
 
type
  TVTDragManager = class(TInterfacedObject, IDropSource)
  public
    constructor Create(AOwner: TComponent); virtual;
    destructor Destroy; override;

    function GiveFeedback(Effect: Integer): HResult; stdcall;
    function QueryContinueDrag(EscapePressed: BOOL; KeyState: Integer): HResult; stdcall;
  end;
 
 
constructor TVTDragManager.Create(AOwner: TComponent);
begin
  inherited Create;
end;

destructor TVTDragManager.Destroy;
begin
  WriteLn('Destroy');
  inherited Destroy;
end;

function TVTDragManager.GiveFeedback(Effect: Integer): HResult;
begin

end;

function TVTDragManager.QueryContinueDrag(EscapePressed: BOOL; KeyState: Integer
  ): HResult;
begin

end;    
     
var
  Manager: TVTDragManager;
begin
  Manager := TVTDragManager.Create(nil);
  WriteLn('Start');
  //Manager.Free;
end.


_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Usage of TInterfacedObject. Automatic release?

Cesar Romero-2
var
  Manager: TVTDragManager;

It should be Interface.

like

var
  Manager: IDropSource;

or other implemented interface

[]s


Cesar Romero

Luiz Americo Pereira Camara escreveu:

> I'm tracing a memory leak in a Delphi ported component and found that
> the culprit was a TInterfacedObject descendant that was not being freed.
>
> I found that if  i directly call Free or _Release the memory leak
> vanishes (Delphi code does not call these).
>
> My question is: is TInterfacedObject descendants supposed to be freed
> automatically or should i call Free directly?
>
> See attached example.
>
> Using fpc 2.1.4 under win32 XP SP2.
>
> Luiz
> ------------------------------------------------------------------------
>
> _______________________________________________
> fpc-pascal maillist  -  [hidden email]
> http://lists.freepascal.org/mailman/listinfo/fpc-pascal

_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Usage of TInterfacedObject. Automatic release?

Luiz Americo Pereira Camara-2
Cesar Romero wrote:

> var
>  Manager: TVTDragManager;
>
> It should be Interface.
>
> like
>
> var
>  Manager: IDropSource;
>
> or other implemented interface
Thanks. You are right. Using Manager as IDropSource there's no leak in
the test program.

Unfortunately in the original code Manager is already an interface but
still leaking memory. It seems that is a more complex bug.

Thanks anyway. If i find how to isolate this bug i will report it. For
now using _Release.

Luiz
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Usage of TInterfacedObject. Automatic release?

Joao Morais
Luiz Americo Pereira Camara wrote:

> Unfortunately in the original code Manager is already an interface but
> still leaking memory. It seems that is a more complex bug.

If you have two or more interfaces whose members point each other in a
circular reference scenario, without using weak reference, you will have
memory leakage.

--
Joao Morais

_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Usage of TInterfacedObject. Automatic release?

Luiz Americo Pereira Camara-2
Joao Morais wrote:
> Luiz Americo Pereira Camara wrote:
>
>> Unfortunately in the original code Manager is already an interface
>> but still leaking memory. It seems that is a more complex bug.
>
> If you have two or more interfaces whose members point each other in a
> circular reference scenario, without using weak reference, you will
> have memory leakage.

Thanks.
I don't have any experience with interfaces but is quite possible that
this is the case. The component i'm porting is the TVirtualTreeView that
uses extensively interfaces/COM and this part of the code is a bit complex.

Anyway, calling manually _Release avoid the leak and i will stay with it
for now (I hope did not break anything).

Luiz

_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Usage of TInterfacedObject. Automatic release?

Cesar Romero-2
Luiz Americo,
> Anyway, calling manually _Release avoid the leak and i will stay with
> it for now (I hope did not break anything).

It can work for that case, but I think that is not a good ideia.

If you are using a TInterfacedObject descendant, it will call the
_Release when out of context, so in that especific case seems to never
go out of context, and then you _realease call works, but in other place
it can go out of context before, and the you will have others AV.

- As Joao suggest, the best way is create a weak reference, where when 2
objects need to reference each other one should use Pointer to hold one
reference
- Avoid circular reference
- Create a Finalization method that will set one of the references to "nil"

[]s


Cesar Romero

_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Usage of TInterfacedObject. Automatic release?

Joao Morais
Cesar Romero wrote:

> - Create a Finalization method that will set one of the references to "nil"

Is this mandatory? I thought the compiler creates this code for global
vars as well as it creates for local vars.

--
Joao Morais
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Usage of TInterfacedObject. Automatic release?

Luiz Americo Pereira Camara-2
In reply to this post by Cesar Romero-2
Cesar Romero wrote:

> Luiz Americo,
>> Anyway, calling manually _Release avoid the leak and i will stay with
>> it for now (I hope did not break anything).
>
> It can work for that case, but I think that is not a good ideia.
>
> If you are using a TInterfacedObject descendant, it will call the
> _Release when out of context, so in that especific case seems to never
> go out of context, and then you _realease call works, but in other
> place it can go out of context before, and the you will have others AV.
>
> - As Joao suggest, the best way is create a weak reference, where when
> 2 objects need to reference each other one should use Pointer to hold
> one reference
> - Avoid circular reference

The problem here is that i'm using a third party design. And the code
complexity allied with my inexperience with interfaces/COM makes the
chance of  breaking things big.

Anyway i did not find, with my limitations, any circular references
between interfaces. Exists a circular reference but is between an
Interface (IVTDragManager) and a TVirtualTreeView(TCustomControl)
already used as a weak reference (TObject).

The design is more or less the following:

TBaseVirtualTree holds a reference to a IVTDragManager.
TVTDragManager (IVTDragManager) holds a reference to TBaseVirtualTree
stored in a TObject field
TVTDragManager also holds reference for IDataObject (TVTDataObject) and
IDropTargetHelper (returned by a win32 call)
TVTDataObject holds a reference to TBaseVirtualTree as a TObject

There are two circular references:
TVTDragManager <-> TBaseVirtualTree
TBaseVirtualTree <-> TVTDragManager > TVTDataObject > TBaseVirtualTree

if someone is interested. the code is here:
http://lazarus-ccr.svn.sourceforge.net/viewvc/lazarus-ccr/components/virtualtreeview-unstable/units/win32/virtualdragmanager.pas?view=log
> - Create a Finalization method that will set one of the references to
> "nil"
Its already done. But the problem is that this finalization code is  not
called (is inside the Interface Destroy). And setting the interface
reference to nil inside TBaseVirtualTree.Destroy does not help.

Luiz

 
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Usage of TInterfacedObject. Automatic release?

Luiz Americo Pereira Camara-2
In reply to this post by Cesar Romero-2
Cesar Romero wrote:

> Luiz Americo,
>> Anyway, calling manually _Release avoid the leak and i will stay with
>> it for now (I hope did not break anything).
>
> It can work for that case, but I think that is not a good ideia.
>
> If you are using a TInterfacedObject descendant, it will call the
> _Release when out of context, so in that especific case seems to never
> go out of context, and then you _realease call works, but in other
> place it can go out of context before, and the you will have others AV.
>

I just  realized that the current design is relatively  safe. If the
interface goes out of context before the call to _Release, the
referenced object will be set to nil. This will prevent a second call to
_Release. Is not the ideal but i'll stay with it for now.

Thanks for Cesar and Joao for all that info.

Luiz
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Usage of TInterfacedObject. Automatic release?

Cesar Romero-2
In reply to this post by Joao Morais
Joao Morais escreveu:
> Cesar Romero wrote:
>
>> - Create a Finalization method that will set one of the references to
>> "nil"
>
> Is this mandatory? I thought the compiler creates this code for global
> vars as well as it creates for local vars.
>
Just a workaround to resolve circular references issues.

[]s

Cesar
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Usage of TInterfacedObject. Automatic release?

Marc Weustink
In reply to this post by Luiz Americo Pereira Camara-2
Luiz Americo Pereira Camara wrote:

> Cesar Romero wrote:
>> Luiz Americo,
>>> Anyway, calling manually _Release avoid the leak and i will stay with
>>> it for now (I hope did not break anything).
>>
>> It can work for that case, but I think that is not a good ideia.
>>
>> If you are using a TInterfacedObject descendant, it will call the
>> _Release when out of context, so in that especific case seems to never
>> go out of context, and then you _realease call works, but in other
>> place it can go out of context before, and the you will have others AV.
>>
>> - As Joao suggest, the best way is create a weak reference, where when
>> 2 objects need to reference each other one should use Pointer to hold
>> one reference
>> - Avoid circular reference
>
> The problem here is that i'm using a third party design. And the code
> complexity allied with my inexperience with interfaces/COM makes the
> chance of  breaking things big.
>
> Anyway i did not find, with my limitations, any circular references
> between interfaces. Exists a circular reference but is between an
> Interface (IVTDragManager) and a TVirtualTreeView(TCustomControl)
> already used as a weak reference (TObject).
>
> The design is more or less the following:
>
> TBaseVirtualTree holds a reference to a IVTDragManager.
> TVTDragManager (IVTDragManager) holds a reference to TBaseVirtualTree
> stored in a TObject field
> TVTDragManager also holds reference for IDataObject (TVTDataObject) and
> IDropTargetHelper (returned by a win32 call)
> TVTDataObject holds a reference to TBaseVirtualTree as a TObject
>
> There are two circular references:
> TVTDragManager <-> TBaseVirtualTree
> TBaseVirtualTree <-> TVTDragManager > TVTDataObject > TBaseVirtualTree

Circular references of object is not the problem, you should look at
circular references of interfaces.

However, I don't expect that this is the case for the virtualtree. THat
you need a _release for fpc and not for delphi sounds like a bug in fpc.
(only where)
Normally you never ever have to call _release yourself (unless you
called _addref yourself)

Marc

_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Usage of TInterfacedObject. Automatic release?

Graeme Geldenhuys-2
In reply to this post by Luiz Americo Pereira Camara-2
On 5/26/07, Luiz Americo Pereira Camara <[hidden email]> wrote:

> I don't have any experience with interfaces but is quite possible that
> this is the case. The component i'm porting is the TVirtualTreeView that
> uses extensively interfaces/COM and this part of the code is a bit complex.

The virtual treeview has already been ported twice by two different
people. Any particular reason you are doing it again?

See the link for one of the ports...
http://tinyurl.com/2uxewm

The other one was based on a newer version of VT, but I don't think
the port was 100% complete yet.


--
Graeme Geldenhuys

General error, hit any user to continue.
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Usage of TInterfacedObject. Automatic release?

Luiz Americo Pereira Camara-2
Graeme Geldenhuys wrote:
>
> The virtual treeview has already been ported twice by two different
> people. Any particular reason you are doing it again?
>
> See the link for one of the ports...
> http://tinyurl.com/2uxewm
>
> The other one was based on a newer version of VT, but I don't think
> the port was 100% complete yet.
See https://luipack.bountysource.com/wiki/virtualtreeview . There you
will find the port roadmap and a link for the reasons of a new port.

The new port is not complete yet. But fully working both in win32 and
gtk (1 and 2). Most of the bugs of the first port were fixed (horizontal
scrolling, checkboxes images, editors position, header resize) and even
a delphi bug was fixed.

Luiz

_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Usage of TInterfacedObject. Automatic release?

Luiz Americo Pereira Camara-2
In reply to this post by Marc Weustink
Marc Weustink wrote:

> Luiz Americo Pereira Camara wrote:
>> Cesar Romero wrote:
>>> Luiz Americo,
>>>> Anyway, calling manually _Release avoid the leak and i will stay
>>>> with it for now (I hope did not break anything).
>>>
>>> It can work for that case, but I think that is not a good ideia.
>>>
>>> If you are using a TInterfacedObject descendant, it will call the
>>> _Release when out of context, so in that especific case seems to
>>> never go out of context, and then you _realease call works, but in
>>> other place it can go out of context before, and the you will have
>>> others AV.
>>>
>>> - As Joao suggest, the best way is create a weak reference, where
>>> when 2 objects need to reference each other one should use Pointer
>>> to hold one reference
>>> - Avoid circular reference
>>
>> The problem here is that i'm using a third party design. And the code
>> complexity allied with my inexperience with interfaces/COM makes the
>> chance of  breaking things big.
>>
>> Anyway i did not find, with my limitations, any circular references
>> between interfaces. Exists a circular reference but is between an
>> Interface (IVTDragManager) and a TVirtualTreeView(TCustomControl)
>> already used as a weak reference (TObject).
>>
>> The design is more or less the following:
>>
>> TBaseVirtualTree holds a reference to a IVTDragManager.
>> TVTDragManager (IVTDragManager) holds a reference to TBaseVirtualTree
>> stored in a TObject field
>> TVTDragManager also holds reference for IDataObject (TVTDataObject)
>> and IDropTargetHelper (returned by a win32 call)
>> TVTDataObject holds a reference to TBaseVirtualTree as a TObject
>>
>> There are two circular references:
>> TVTDragManager <-> TBaseVirtualTree
>> TBaseVirtualTree <-> TVTDragManager > TVTDataObject > TBaseVirtualTree
>
> Circular references of object is not the problem, you should look at
> circular references of interfaces.
>
As i explained i did not found interface circular references. (Although
does not mean it does not exists ;-))
> However, I don't expect that this is the case for the virtualtree.
> THat you need a _release for fpc and not for delphi sounds like a bug
> in fpc. (only where)
This was my first idea. And now gains force again. The problem is that
the bug can not be isolated easily.

Another option is that there's a mem leak in delphi also. Does some one
know how to trace a mem leak in delphi?

Luiz
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Usage of TInterfacedObject. Automatic release?

Cesar Romero-2
Luiz Americo,
> This was my first idea. And now gains force again. The problem is that
> the bug can not be isolated easily.
>
> Another option is that there's a mem leak in delphi also. Does some
> one know how to trace a mem leak in delphi?
>
Use FastMM with FullDebugOptions

[]s


Cesar Romero
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Usage of TInterfacedObject. Automatic release?

Graeme Geldenhuys-2
In reply to this post by Luiz Americo Pereira Camara-2
On 5/28/07, Luiz Americo Pereira Camara <[hidden email]> wrote:

> Graeme Geldenhuys wrote:
> >
> > The virtual treeview has already been ported twice by two different
> > people. Any particular reason you are doing it again?
> >
> > See the link for one of the ports...
> > http://tinyurl.com/2uxewm
> >
> > The other one was based on a newer version of VT, but I don't think
> > the port was 100% complete yet.
> See https://luipack.bountysource.com/wiki/virtualtreeview . There you
> will find the port roadmap and a link for the reasons of a new port.

Ah yes, that's the other port of Virtual Treeview I was talking about...  :-)
I just couldn't remember the url.


--
Graeme Geldenhuys

General error, hit any user to continue.
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Usage of TInterfacedObject. Automatic release?

Luiz Americo Pereira Camara-2
In reply to this post by Cesar Romero-2
Cesar Romero wrote:
> Luiz Americo,
>> This was my first idea. And now gains force again. The problem is
>> that the bug can not be isolated easily.
>>
>> Another option is that there's a mem leak in delphi also. Does some
>> one know how to trace a mem leak in delphi?
>>
> Use FastMM with FullDebugOptions

I did and no signal of mem leak (tested OLE demo).

Probably is a fpc bug.

If someone looked at the code, noticed that i splitted the dragmanager
code in a separate unit and that i modified some things. But this is not
the culprit since if you compile with UseExternalDragManager undefined
you will end with exactly the same Delphi code (regarding VTVDragmanager
design). I updated the svn to compile with this symbol undefined using
fpc 2.1.4.

If someone is interested in investigate further some notes

* Is necessary to use fpc 2.1.4 to compile without
UseExternalDragManager since ActiveX in 2.0.4 in buggy. Otherwise 2.0.4
is fine.
* Remove the _Release call in Destroy.
* Compile a example where OLE dnd is activated (OLE) under windows

Luiz
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/mailman/listinfo/fpc-pascal