Dealing with circular strong references in Delphi

Question

I got two classes (in my example TObject1 and TObject2) which know each other via interfaces (IObject1, IObject2). As you probably know in Delphi this will lead to a memory leak as both reference counter will always stay above zero. The usual solution is declaring one reference as weak. This works in most cases because you usually know which one will be destroyed first or don't necessarily need the object behind the weak reference once it is destroyed.

This said I tried to solve the problem in a manner that both objects stay alive until both aren't referenced anymore: (Delphi 10.1 required as I use the [unsafe] attribute)

program Project14;

{$APPTYPE CONSOLE}

uses
  System.SysUtils;

type
  IObject2 = interface;

  IObject1 = interface
    ['{F68D7631-4838-4E15-871A-BD2EAF16CC49}']
    function GetObject2: IObject2;
  end;

  IObject2 = interface
    ['{98EB60DA-646D-4ECF-B5A7-6A27B3106689}']
  end;

  TObject1 = class(TInterfacedObject, IObject1)
    [unsafe] FObj2: IObject2;
    constructor Create;
    destructor Destroy; override;

    function GetObject2: IObject2;
  end;

  TObject2 = class(TContainedObject, IObject2)
    [unsafe] FObj1: IObject1;
    constructor Create(aObj1: IObject1);
    destructor Destroy; override;
  end;

constructor TObject1.Create;
begin
  FObj2 := TObject2.Create(Self);
end;

destructor TObject1.Destroy;
begin
  TContainedObject(FObj2).Free;
  inherited Destroy;
end;

function TObject1.GetObject2: IObject2;
begin
  Result := FObj2;
end;

constructor TObject2.Create(aObj1: IObject1);
begin
  inherited Create(aObj1);
  FObj1 := aObj1;
end;

destructor TObject2.Destroy;
begin
  inherited Destroy;
end;

function Test1: IObject1;
var
  x: IObject2;
begin
  Result := TObject1.Create;
  x := Result.GetObject2;
end;

function Test2: IObject2;
var
  x: IObject1;
begin
  x := TObject1.Create;
  Result := x.GetObject2;
end;

var
  o1: IObject1;
  o2: IObject2;
begin
  try
    o1 := Test1();
    o2 := Test2();
  except
    on E: Exception do
      Writeln(E.ClassName, ': ', E.Message);
  end;
end.

This does work as it is.. function Test1 and Test2 each create one instance of TObject1 and TObject2 referencing each other and all instances get destroyed once o1 and o2 go out of scope. The solution is based on TContainedObject which forwards the refcounting to the "controller" (TObject1 in this case).

Now I know this solution has flaws, and this is where my questions start:

• "TContainedObject(FObj2).Free;" smells a bit, but I don't have a better solution as I need to use an interface to reference to TObject2 (the productive code contains a few inheritance on this end). Any ideas to clean it up?
• you easily forget to declare all reference between the 2 classes as weak and ..
• a similar problem starts to raise with more classes: Having TObject3 which is referenced by one and references the other: memory leak. I could handle it by letting it descent from TContainedObject too but with legacy code this might not be an easy task.

I have the feeling this solution can't be applied universally and hoping for one which can - or maybe an answer that will describe why it is so hard or even impossible to have an easy to use 100%-solution to manage such situations. Imho it can't be to complicated to have a finite amount of object which destroy each other once they are not referenced from out of their domain without having to carefully think about every reference within this domain.

Answer 1

If you want to keep both objects alive or dead together, the surely they are one single object. OK, I get that both may be developed by different people, so then I would make them both members of one super-object that is reference counted, like this

type
  TSuperobject = class( TInterfaceObject, IObject1, iObject2 )
  private
    fObject1 : TObject1;
    fObject2 : TObject2;
  public
    constructor Create;
    destructor Destroy;
    function GetObject2: IObject2;
    etc.
  end;

etc.

The details should be obvious. Any reference to object1 or object2 must reference the owning object( superobject.object1 etc.), so object1 and object2 themselves do not need to be reference counted - i.e. they can be regular objects, not interfaced objects, but it actually doesn't matter if they are reference counted because the owner will always add 1 to the reference count (in that case you may not need the destructor in the superobject). If you are leaving object1 and object2 as referenced objects make their refence to each other both weak.

Answer 2

Don't use unsafe
[unsafe] should not be used in normal code.
It is really a hack to the used if you don't want the compiler to do reference counting on interfaces.

Use weak instead
If for some reason you must have circular references then use a [weak] attribute on one of the references and declare the other reference as usual.

In your example it would look like this:

  TParent = class(TInterfacedObject, IParent)
    FChild: IChild;   //normal child
    constructor Create;
    function GetObject2: IChild;
  end;

  TChild = class(TContainedObject, IChild)
    //reference from the child to the parent, always [weak] if circular.
    [weak] FObj1: IParent;   
    constructor Create(const aObj1: IParent);
  end;

Now there is no need to do anything special in the destructors, so these can be omitted.
The compiler tracks all weak references and sets them to nil when the reference count of the referenced interface reaches zero.
And all this is done in a thread-safe manner.
However the weak reference itself does not increase the reference count.

When to use unsafe
This is in contrast to the unsafe reference, where no tracking and no reference counting at all takes place.

You would use an [unsafe] reference on an interfaced type that is a singleton, or one that has disabled reference counting.
Here the ref count is fixed at -1 in any case, so the calling of addref and release is an unneeded overhead.
Putting the [unsafe] eliminates that silly overhead.
Unless your interfaces override _addref and _release do not use [unsafe].

Pre Berlin alternative
Pre Berlin there is no [weak] attribute outside the NexGen compilers.
If you are running Seattle, 2010 or anything in between the following code would do {almost} the same.
Although I'm unsure if this code might not fall victim to race conditions in multithreaded code.
If that's a concern for you feel free to raise a flag and I'll investigate.

  TParent = class(TInterfacedObject, IParent)
    FChild: IChild;   //normal child
    constructor Create;
    function GetObject2: IChild;
  end;

  TChild = class(TContainedObject, IChild)
    //reference from the child to the parent, always [weak] if circular.
    FObj1: TParent;   //not an interface will not get refcounted.  
    constructor Create(const aObj1: IParent);
    destructor Destroy; override;
  end;

  constructor TChild.Create(const aObj1: IParent);
  begin
    inherited Create;
    FObject1:= (aObj1 as TParent);
  end;

 destructor TParent.Destroy;
 begin
   if Assigned(FChild) then FChild.InvalidateYourPointersToParent(self);
   inherited;
 end;

This will also ensure the interfaces get properly disposed, however now TChild.FObject1 will not automatically get nilled. You might be able to put code in the destructor of the TParent to visit all its children and inform them as in the code shown.
If one of the participants in the circular reference can't inform its weakly linked counterparts then you'll need to setup some other mechanism to nil those weak references.

Answer 3

It looks like you want both objects to share their reference count. You could do that by letting a third object (TPair) handle the reference counting. A nice way to accomplish this is by using the implements keyword. You can choose to keep this third object hidden, or to interact with that as well.

With the code below you can either create a TPairChildA, a TPairChildB or their 'parent' TPair. Any of them will create the others when needed and all created objects will be kept alive until none are referenced anymore. You can of course add interfaces like your IObject1 to the objects, but I kept them out for simplicity.

unit ObjectPair;

interface

type
  TPairChildA = class;
  TPairChildB = class;

  TPair = class( TInterfacedObject )
  protected
    FChildA : TPairChildA;
    FChildB : TPairChildB;

    function GetChildA : TPairChildA;
    function GetChildB : TPairChildB;
  public
    destructor Destroy; override;

    property ChildA : TPairChildA read GetChildA;
    property ChildB : TPairChildB read GetChildB;
  end;

  TPairChild = class( TObject , IInterface )
  protected
    FPair : TPair;

    property Pair : TPair read FPair implements IInterface;
  public
    constructor Create( APair : TPair = nil ); virtual;
  end;

  TPairChildA = class( TPairChild )
  protected
    function GetSibling : TPairChildB;
  public
    constructor Create( APair : TPair = nil ); override;

    property Sibling : TPairChildB read GetSibling;
  end;

  TPairChildB = class( TPairChild )
  protected
    function GetSibling : TPairChildA;
  public
    constructor Create( APair : TPair = nil ); override;

    property Sibling : TPairChildA read GetSibling;
  end;

implementation

//==============================================================================
// TPair

destructor TPair.Destroy;
begin
  FChildA.Free;
  FChildB.Free;
  inherited;
end;

function TPair.GetChildA : TPairChildA;
begin
  if FChildA = nil then
    FChildA := TPairChildA.Create( Self );
  Result := FChildA;
end;

function TPair.GetChildB : TPairChildB;
begin
  if FChildB = nil then
    FChildB := TPairChildB.Create( Self );
  Result := FChildB;
end;

// END TPair
//==============================================================================
// TPairChild

constructor TPairChild.Create( APair : TPair = nil );
begin
  if APair = nil then
    FPair := TPair.Create
  else
    FPair := APair;
end;

// END TPairChild
//==============================================================================
// TPairChildA

constructor TPairChildA.Create( APair : TPair = nil );
begin
  inherited;
  FPair.FChildA := Self;
end;

function TPairChildA.GetSibling : TPairChildB;
begin
  Result := FPair.ChildB;
end;

// END TPairChildA
//==============================================================================
// TPairChildB

constructor TPairChildB.Create( APair : TPair = nil );
begin
  inherited;
  FPair.FChildB := Self;
end;

function TPairChildB.GetSibling : TPairChildA;
begin
  Result := FPair.ChildA;
end;

// END TPairChildB
//==============================================================================

end.

A usage example:

procedure TForm1.Button1Click( Sender : TObject );
var
  objA : TPairChildA;
  ifA , ifB : IInterface;
begin
  objA := TPairChildA.Create;
  ifA := objA;
  ifB := objA.Sibling;
  ifA := nil;
  ifB := nil; // This frees all three objects.
end;

Answer 4

You are solving the wrong problem here.

Your actual problem is not in strong - weak references nor how your solution can be improved. Your problem is not in how to achieve, but in what you are achieving (want to achieve).

To directly address your questions first:

• "TContainedObject(FObj2).Free;" smells a bit, but I don't have a better solution as I need to use an interface to reference to TObject2 (the productive code contains a few inheritance on this end). Any ideas to clean it up?

You cannot do much here. You must call Free on FObj2 because TContainedObject is not managed class itself.

• you easily forget to declare all reference between the 2 classes as weak and ..

You cannot do anything here either. It comes with the territory. If you want to use ARC you have to think about circular references.

• a similar problem starts to raise with more classes: Having TObject3 which is referenced by one and references the other: memory leak. I could handle it by letting it descent from TContainedObject too but with legacy code this might not be an easy task.

You cannot do much here either. If your design is really what you want to have, then you will just have to deal with its complexities.

---

Now, back to why you are having problems in the first place.

What you want to achieve (and you have done so with your example code) is keeping whole object hierarchy alive by grabbing any of the object references inside that hierarchy.

To rephrase, you have Form and a Button on it and you want to keep Form alive is something holds a Button (because Button itself would not function). Then you want to add Edit to that Form and again keep everything alive if something grabs Edit.

You have few options here.

• Keep this broken design and live with your solution because you have too much code involved and change would be painful. If you do that keep in mind that this is ultimately broken design and don't attempt to repeat it anywhere else.

• If you have hierarchy where TObject1 is root class that holds all else, then refactor it and inherit TObject2 from TInterfacedObject to have its own reference counting and don't grab references to FObj2. Instead grab root TObject1 instance and pass that around, if you really need to.

• This is variation of second approach. If TObject1 is not the root class then create additional wrapper class containing all instances you need and pass that one around.

Last two solutions are far from perfect and they don't deal with fact that you probably have classes that are doing too much or similar. But no matter how bad that code might be, it does not even come close to your current solution. And with time you can slowly change and improve those solutions much easier than with your current one.