Mix interface with objects

[Berocoder 30]

I use Pascal Analyzer. It is a great application for detecting both small and large mistakes that can occur.
Now I got WARN53.
From doc at https://www.peganza.com/PALHelp/index.html?warnings_report.htm

WARN53-Mixing interface variables and objects

 This section reports locations in your code with assignments between objects and interface variables. Normally, unless you really know what you are doing, it is a bad idea to mix interfaces and objects. The reason is that the reference counting mechanism of interfaces can be disturbed, leading to access violations and/or memory leaks.

Example:

My method. TBoldMember is a class in Bold for Delphi

function TModificationHistoryTracker.GetOldLink(ABoldMember: TBoldMember): TBoldObject;
var
  StringRepresentable: IBoldStringRepresentable;
  OldValue: IBoldValue;
begin
  OldValue := ABoldMember.OldValue;
  if Assigned(OldValue) then
  begin
    if not Supports(OldValue, IBoldStringRepresentable, StringRepresentable) then
      raise EModificationHistoryTracker.Create('Unsupported value type.');

    Result := GetApplicationKernel.MetaSystem.GetObjectForIdString(StringRepresentable.asString);
  end
  else
    Result := nil;
end;

I also asked Claude, and it claimed that this could be a problem, as both Bold may free the memory. When OldValue goes out of scope, the reference becomes 0, and another free operation occurs.
So the same memory is freed twice. The solution would be to remove the temporary variable OldValue.

I have to say that I am confused. Is the code OK or bad?

Here is the text from Claude:

The Core Issue: Two Different Memory Management Models

Delphi has two fundamentally different ways of managing object lifetime, and your code is mixing them in a way that can be dangerous.

Object references use manual memory management. When you create an object with TMyObject.Create, you own that object and must explicitly call Free to destroy it. The object exists until you explicitly destroy it, regardless of how many variables point to it.

Interface references use automatic reference counting. When you assign an object to an interface variable, Delphi automatically tracks how many interface variables are pointing to that object. When the last interface reference goes out of scope, Delphi automatically destroys the object.

What's Happening in Your Code

Let's trace through your code step by step:

 

OldValue := ABoldMember.OldValue;

The property ABoldMember.OldValue is declared as returning IBoldValue, which is an interface type. However, internally, the Bold framework is managing actual objects (probably instances of TBoldValue or a descendant class). These objects are owned and managed by the Bold framework itself using its own lifecycle management.

When you assign this to your OldValue variable, something subtle but important happens. Delphi performs an implicit cast from the object to the interface. This cast does several things behind the scenes:

First, Delphi calls QueryInterface on the underlying object to get the IBoldValue interface pointer. Second, and this is the critical part, Delphi increments the reference count on that object. The object now thinks it has one interface reference pointing to it.

Why This Creates a Problem

The Bold framework created and owns that TBoldValue object. It's managing that object's lifetime as part of its domain model. The framework expects to be in control of when that object lives and dies.

But now you've created an interface reference to it. When your OldValue variable goes out of scope at the end of the function, Delphi will automatically decrement the reference count. If that reference count reaches zero, Delphi will attempt to destroy the object.

This creates a conflict. You have two different systems trying to manage the same object's lifetime. The Bold framework thinks it owns the object and will free it when appropriate. The interface reference counting system also thinks it has responsibility for the object. This can lead to several problems:

The object might be destroyed prematurely while Bold still needs it, causing access violations later. Alternatively, if Bold manages to free the object first, your interface reference becomes a dangling pointer, and when it goes out of scope, Delphi tries to decrement the reference count on an already-freed object, causing a crash. In some cases, you might get a memory leak if the reference counting prevents Bold from properly cleaning up.

Why Pascal Analyzer Warns

Pascal Analyzer performs static analysis on your code. It sees that you're taking a property that returns an interface type and storing it in a local interface variable. It recognizes this pattern as potentially dangerous because it's mixing two incompatible memory management strategies.

The analyzer is essentially saying: "I see you're creating a reference-counted interface reference to an object that might be managed by a different system. This could cause lifetime management conflicts."

The Solution

The way to fix this is to avoid creating the intermediate interface variable. Instead, use the property directly where you need it. This way, you're still working with the interface, but you're not creating a stored reference that participates in reference counting:

 

[Anders Melander 2137]

AI;DR

[Dave Nottage 648]

1 hour ago, Berocoder said:

The property ABoldMember.OldValue is declared as returning IBoldValue, which is an interface type. However, internally, the Bold framework is managing actual objects (probably instances of TBoldValue or a descendant class)

"Probably"? 😆

 

I'd be checking exactly what type the ABoldMember.OldValue property is.

[Berocoder 30]

7 hours ago, Dave Nottage said:

"Probably"? 😆

 

I'd be checking exactly what type the ABoldMember.OldValue property is.

It is a property

    property OldValue: IBoldValue read GetOldValue;

implemented here

function TBoldMember.GetOldValue: IBoldValue;
var
  ObjectContents: IBoldObjectContents;
begin
  result := nil;
  if OwnedByObject and not OwningObject.BoldObjectIsNew then
  begin
    ObjectContents := BoldSystem.fOldValueHandler.OldValues.GetObjectContentsByObjectId(OwningObject.BoldObjectLocator.BoldObjectID) as IBoldObjectContents;
    if assigned(ObjectContents) then
    begin
      result := ObjectContents.ValueByIndex[BoldMemberRTInfo.index];
    end;
  end;
end;

Then ValueByIndex is also a property

    property ValueByIndex[I: Integer]: IBoldValue read GetValueByIndex;

And implementation

function TBoldFreeStandingObjectContents.GetValueByIndex(I: Integer): IBoldValue;
begin
  if i < Length(fMemberList) then
    result := fMemberList[i]
  else
    result := nil;
end;

fMemberlist is an array:

    fMemberList: array of TBoldFreeStandingValue;

freed in OnDestroy

destructor TBoldFreeStandingObjectContents.Destroy;
var
  i: integer;
begin
  for i := 0 to Length(fMemberList) - 1 do
    FreeAndNil(fMemberList[i]);
  inherited;
end;

That is how far I could go for now
 

[Kas Ob. 160]

10 hours ago, Berocoder said:

Is the code OK or bad?

Bad, 

 

The thing is and Claude is right here, but failed to point that this function "TModificationHistoryTracker.GetOldLink" itself shouldn't exist in first place (well in its current parameters and return result), if you want to check of OldValue then make it pass the OldValue as parameter and return an interface, then the caller would check if the interface is exist and resolve it to the object, in other words you didn't need this check against OldLink then you didn't need to jump through their objects.

 

In other words, the handling should be Interface to Interface, and never to do Interface to object to interface, so you can either

1) introduce another interface as old link manager with one function GetOldLink, this will solve this once and for all, or

2) refactor that TModificationHistoryTracker.GetOldLink into interface as parameter to return interface.

[Berocoder 30]

8 hours ago, Kas Ob. said:

Bad, 

 

The thing is and Claude is right here, but failed to point that this function "TModificationHistoryTracker.GetOldLink" itself shouldn't exist in first place (well in its current parameters and return result), if you want to check of OldValue then make it pass the OldValue as parameter and return an interface, then the caller would check if the interface is exist and resolve it to the object, in other words you didn't need this check against OldLink then you didn't need to jump through their objects.

 

In other words, the handling should be Interface to Interface, and never to do Interface to object to interface, so you can either

1) introduce another interface as old link manager with one function GetOldLink, this will solve this once and for all, or

2) refactor that TModificationHistoryTracker.GetOldLink into interface as parameter to return interface.

Actually I found more info now in Bold. The interface IBoldValue has a baseclass TBoldNonRefCountedObject with this implementation.
So referencecounting is disabled. I think this change a lot

  TBoldNonRefCountedObject = class(TBoldInterfacedObject)
  protected
    function _AddRef: Integer; override;
    function _Release: Integer; override;
  end;


function TBoldNonRefCountedObject._AddRef: Integer;
begin
  Result := -1;
end;

function TBoldNonRefCountedObject._Release: Integer;
begin
  Result := -1;
end;

 

[A.M. Hoornweg 161]

I do sometimes mix objects and interfaces. 

 

But only when I have an object X that implements an interface Y, when the object requires additional configuration or dependencies that are not part of interface Y and that can not be simply injected into the constructor.  

 

So I create the object, perform the necessary configuration and then assign it to the interface that will manage the lifetime of the object.  Just in case, I make the object pointer "disappear" immediately after the assignment.  

Function test: iInterfaceY;
Var t: tInterfacedObjectX; 
begin
  t:=tInterfacedobjectX.Create;
  DoSomeComplexInitialization(t);  
  result:=t;  // interface manages the lifetime from now on
end;

 

 

[Kas Ob. 160]

20 minutes ago, A.M. Hoornweg said:

I do sometimes mix objects and interfaces. 

I think we all did that and still do that, and that why Interfaces are pain in the back to maintain in the long run, it is really hard to switch from one style in designing to another in the same language, example

 

21 minutes ago, A.M. Hoornweg said:

But only when I have an object X that implements an interface Y, when the object requires additional configuration or dependencies that are not part of interface Y and that can not be simply injected into the constructor.  

 

So I create the object, perform the necessary configuration and then assign it to the interface that will manage the lifetime of the object.  Just in case, I make the object pointer "disappear" immediately after the assignment.  

Function test: iInterfaceY;
Var t: tInterfacedObjectX; 
begin
  t:=tInterfacedobjectX.Create;
  DoSomeComplexInitialization(t);  
  result:=t;  // interface manages the lifetime from now on
end;

Right, and as i said i think every one did that at some point because it look right, then later in time will regret it and start to refactor it.

 

My rule of thumb here is that, if you are going with using interfaces like TInterfacedObject then all its public should be only interfaces and other types but never objects, this will enforce best practice and keep the compiler on the same page with you, handling the object should be in protected and even better in private, passing TInterfacedObject should always with as interface parameter, with the one interface that you need to operate, but never in general or all, just the one, if there is no this exact one then its time to add one, this what makes best practice with interfaces waste of time and code, in other words in code like the above (where i saw similar and wrote many times), i had to refactor it into something like IComplexInitializer belong to special object that do the initializing, its object will do handle the object in protected method, again never in public.

 

And that why i have interfaces, to do them right, you have to overengineer and need way more deep thinking to get it done nicely, clear and maintainable code.

[A.M. Hoornweg 161]

Ah, but I very often need to implement new interfaces in existing classes that I wrote many years ago. Classes that weren't interface based in the first place.

 

For example, some classes that I originally wrote for an entirely different purpose (to import numerical Ascii, Excel, WitsML and LAS files into a database) are now also used to preview such files in a 2-dimensional grid by implementing a common interface that exposes the data as a virtual grid consisting of rows and columns. I could change the base class of some of these classes into tInterfacedObject, in others I had to implement the _AddRef and _Release logic manually. 

 

But basically, adding interfaces to existing classes is dead easy.

 

 

[Uwe Raabe 2233]

To connect both worlds I often use this pattern:

type
  TMyCurrentClass = class
  public
    procedure MyMethod;
  end;

type
  IMyInterface = interface
    ['{B3D71E7A-9066-4407-BFE2-B4BB61221954}']
    procedure MyMethod;
  end;

type
  TMyCurrentClassWrapper = class(TInterfacedObject, IMyInterface)
  private
    FMyInstance: TMyCurrentClass;
    FOwnsInstance: Boolean;
    function GetMyInstance: TMyCurrentClass;
  public
    constructor Create(AMyInstance: TMyCurrentClass = nil; AOwnsInstance: Boolean = False);
    destructor Destroy; override;
    property MyInstance: TMyCurrentClass read GetMyInstance implements IMyInterface;
  end;

...

constructor TMyCurrentClassWrapper.Create(AMyInstance: TMyCurrentClass = nil; AOwnsInstance: Boolean = False);
begin
  inherited Create;
  FMyInstance := AMyInstance;
  FOwnsInstance := AOwnsInstance;
end;

destructor TMyCurrentClassWrapper.Destroy;
begin
  FMyInstance.Free;
  inherited;
end;

function TMyCurrentClassWrapper.GetMyInstance: TMyCurrentClass;
begin
  if FMyInstance = nil then begin
    FMyInstance := TMyCurrentClass.Create;
    FOwnsInstance := True;
  end;
  Result := FMyInstance;
end;

It allows class-based and reference counted interface-based usage without fiddling around with the base class. The key point is that you cannot retrieve the interface from an instance of TMyCurrentClass avoiding most of the mix problems.

 

The scenario with giving a non-owned instance with the constructor still is a fragile usage though. One has to guarantee that the instance is valid for the whole lifetime of the wrapper.