Has anyone experienced a memory leak problem using delegates/implements resource?

-
Has anyone experienced a memory leak problem using delegates/implements resource?

Follows a sample code:

https://gist.github.com/dipold/8ad9b760bcb1942c9bf99b4716b09a0d

Can this be considered a compiler bug?

I'm using Delphi XE8 and I'm following data from the post below:
http://objectpascalprogramming.com/posts/objetos-agregados/

Comments

  1. David HeffernanDecember 12, 2016 at 3:57 AM

    It's not a bug, it's by design. More here: stackoverflow.com - Delphi interface implements

    ReplyDelete
  2. Chris RollistonDecember 12, 2016 at 4:07 AM

    Possibly - if you assign the newly-constructed TMyApp to a reference typed to a non-aggregated interface (i.e. IInterface in this case) first, then there's no reference count mixup, in this scenario:

    type //add GUID
    IValue = interface
    ['{C95A3CA8-5129-4013-A820-1A2B6CF0FA90}']
    function AsString: string;
    end;

    procedure ExecuteMyAppAsInterface;
    var
    I: IInterface;
    V: IValue;
    begin
    WriteLn;
    WriteLn('MyAppAsInterface:');
    I := TMyApp.Create(20);
    V := I as IValue;
    WriteLn(V.AsString);
    end;

    However, I've never understood why people think this feature so wonderful. To be used properly, convert TIntegerValue to descend from TAggregatedObject, use the object not the interface type in TMyApp, and free the object in TMyApp.Destroy:

    //...
    TIntegerValue = class(TAggregatedObject, IValue)
    private
    FValue: Integer;
    public
    constructor Create(const AController: IInterface;
    Value: Integer);
    //...

    TMyApp = class(TInterfacedObject, IValue)
    private
    FValue: TIntegerValue;
    public
    constructor Create(Value: Integer);
    destructor Destroy; override;
    property Value: TIntegerValue
    read FValue implements IValue;
    end;

    //...

    constructor TMyApp.Create(Value: Integer);
    begin
    inherited Create;
    FValue := TIntegerValue.Create(Self, Value);
    end;

    constructor TIntegerValue.Create(
    const AController: IInterface; Value: Integer);
    begin
    inherited Create(AController);
    FValue := Value;
    end;

    //...

    destructor TMyApp.Destroy;
    begin
    FValue.Free;
    inherited Destroy;
    end;

    Doing things this way means there will only ever be one IInterface/IUnknown implementation around, which is how things should be.

    ReplyDelete
  3. Christopher WosinskiDecember 12, 2016 at 5:39 AM

    IMHO the "implements" feature is broken by design. A class which is used as an implementation delegate needs to derive from TAggregateObject. It has to know about the context it is supposed to live in during compile time. It can't be used as a standalone object referenced as an interface since it has to reflect the calls to _AddRef and _Release back to its owner. That's violating the SOLID principles. The idea of having replacable concrete interface implementations is nice but the way of doing it via TAggregatedObject and the need of freeing it manually is producing a lot of trouble. I recommend to not use it ever.

    ReplyDelete
  4. Rafael DipoldDecember 12, 2016 at 8:14 AM

    Exactly Christopher.

    The link I mentioned, uses decoration and composition to workaround this. So each Interface in your code will have one, and only one, aggregate implementation that can be used with any other "normal class" that implements the same Interface:

    Https://gist.github.com/dipold/a91e8260edf3c588d392932cd863a985

    But this still sounds weird..

    ReplyDelete

Post a Comment