16

In some part of my application, I have the situation where I receive an interface which I know to be an object, albeit I don't know the exact class. I have to store that object in an interface-type variable.

Eventually, I might receive another instance of that type, and the first one must be discarded and replaced with the new one. For that, I need to free the memory that interfaced object uses (my interface provides an AsObject method so I can use the TObject methods on it). My problem is, when I want to assign "nil" to that variable again, I get an access violation.

I wrote a small program that reproduces my situation. I post it here to clarify the situation.

program Project1;

{$APPTYPE CONSOLE}

uses
  SysUtils, Classes;

type
   ISomeInterface = interface
      function SomeFunction : String;
      function AsObject : TObject;
   end;

   TSomeClass = class(TComponent, ISomeInterface)
   public
      called : Integer;
      function SomeFunction : String;
      function AsObject : TObject;
   end;

var
   SomeInterface : ISomeInterface;
   i : Integer;

function TSomeClass.SomeFunction : String;
begin
   Result := 'SomeFunction called!';
end;

function TSomeClass.AsObject : TObject;
begin
   Result := Self;
end;

begin
   try
      SomeInterface := nil;

      for i := 1 to 10 do
      begin

         if SomeInterface <> nil then
         begin
            SomeInterface.AsObject.Free;
            SomeInterface := nil;          // <-- Access Violation occurs here
         end;

         SomeInterface := TSomeClass.Create(nil);
         SomeInterface.SomeFunction;       // <-- if commented, Access 
                                           //     Violation does not occur

      end;

   except on e : Exception do
      WriteLn(e.Message);
   end;

end.

So the question is: how can I free that object correctly?

RRUZ
  • 130,998
  • 15
  • 341
  • 467
Pablo Venturino
  • 4,862
  • 4
  • 29
  • 40

4 Answers4

31

Assuming that you have a legitimate reason for doing this (and using TComponent it is quite possible that you do - see end of answer for why), then the problem occurs as a result of changing the reference of an interface variable after you have destroyed the object it currently references.

Any change to an interface reference generates code like this:

  intfA := intfB;

becomes (in simple terms):

  if Assigned(intfA) then
    intfA.Release;

  intfA := intfB;

  if Assigned(intfA) then
    intfA.AddRef;

If you relate that to your code, you should see the problem:

  SomeInterface.AsObject.Free;
  SomeInterface := nil;  

becomes:

SomeInterface.AsObject.Free;

if Assigned(SomeInterface) then
  SomeInterface.Release;

SomeInterface := nil;  

if Assigned(SomeInterface) then
  SomeInterface.AddRef;

So you can see that it is the generated call to Release() that results from assigning NIL to the interface that causes your access violation.

You should also quickly see that there is a simple way to avoid this, simply defer the freeing of the object until after you have NIL'd the interface reference:

obj := SomeInterface.AsObject;
SomeInterface := NIL;
obj.Free;

BUT

The key question here is why are you explicitly free'ing an object that is interfaced (and presumably reference counted).

When you change the code to cache the object reference and NIL the interface before explicitly Free'ing the object, you may find that obj.Free will then cause an access violation as the NIL'ing of the interface reference might itself result in the object being free'd.

The ONLY way to be sure that explicitly free'ing the interfaced object is safe is:

1) That the interfaced object has overridden/reimplemented IUnknown and eliminated the reference counted lifetime management

AND

2) That you have no other interfaced references to that object elsewhere in your code.

If the first of these conditions doesn't make much sense to you then, without wishing to be patronising, this is probably a good sign that you shouldn't be explicitly freeing the object as it is almost certainly being managed by reference count.

Having said that, since you are using an interfaced TComponent class then as long as your TComponent class does not encapsulate a COM Object, then TComponent meets condition #1, so all that then remains is that you ensure the rest of your code meets condition #2.

Deltics
  • 20,843
  • 2
  • 38
  • 67
  • 1
    This is incorrect since `TComponent` does not use reference counting; `AddRef` and `Release` return -1 – David Heffernan Jan 27 '11 at 22:14
  • Excuse me, but it is correct in the general case. TComponent is a special case that meets condition #1 at the end of the answer for conditions when it is safe to explicitly NIL an interfaced object. And since TComponent is involved in the posters case, the suggested change to the sequence of NIL'ing before Free'ing rather than vice versa will solve the problem. Your statement that "this is incorrect" is itself incorrect and misleading imho, though it would perhaps be helpful to be clearer about the relevance of the answer to TComponent specifically, and I shall edit the answer accordingly. – Deltics Jan 27 '11 at 22:28
  • ... and given that the posters code is clearly simplified for the purposes of posting here, there is always the possibility that the specific class involved is using reference counting even though it derives from class with a default IUnknown implementation that does not. That's the beauty of Delphi - you can change these things. Of course, that's also a danger, but... :) – Deltics Jan 27 '11 at 22:34
  • there is no reference counting going on in the OP's code. That's what many of the answers have missed. – David Heffernan Jan 27 '11 at 22:40
  • OP has a memory leak in the code and indicates in the comments that he is experiencing access violations. I don't see the access violations but I do see the memory leak. Might be a difference between D2010 and D7. – Mikael Eriksson Jan 27 '11 at 23:11
  • @David: Should have read your answer before commenting here :) – Mikael Eriksson Jan 27 '11 at 23:13
  • @mikael posted code leaks one instance, the last one, and cannot AV. I believe OP is not running this code. – David Heffernan Jan 27 '11 at 23:16
  • 2
    Many thanks, Deltics. Your suggestion on reversing the freeing-nilling code sounds like it will solve my problem. And it's quite an insight in how interfaces work, I learned lots with your answer. Thanks again. – Pablo Venturino Jan 27 '11 at 23:26
  • Still, I don't have Delphi at home, so I'll have to wait till tomorrow to try this at work. I'll let you know by then. (By the way, this app is legacy code and every class in it is a TComponent). – Pablo Venturino Jan 27 '11 at 23:28
  • 4
    +1. Although you can disable reference counting in the class (by providing implementations of `_AddRef` and `_Release` that simply don't count anything, you cannot disable the compiler's generation of *calls* to the reference-counting functions. And since the functions are always going to be called, you need to make sure that there are no remaining references *before* you destroy the object that the interface variables refer to, even if you're not going to *count* the references. – Rob Kennedy Jan 27 '11 at 23:33
  • what rob said is the explanation for the AV! – David Heffernan Jan 27 '11 at 23:47
  • 1
    What Rob said is what I explained in my answer! ;) – Deltics Jan 28 '11 at 00:17
  • @deltics I can see that now. Sorry. I did a trivial edit so that I could reverse my downvote to an upvote. – David Heffernan Jan 28 '11 at 03:46
  • @deltics This issue is brand new to me because my non ref counted objects don't touch Self in AddRef or Release and thus are safe from this problem. That's why I was blind to it and, wrongly, believed you were talking about reference count provoked calls to Free. – David Heffernan Jan 28 '11 at 03:49
  • 2
    @deltics I'd love to hear more about the bug in TInterfacedObject destructor. – David Heffernan Jan 28 '11 at 03:51
  • @David, I don't believe it matters whether addref/release "touch" self. It was explained to me once before that the pragmas involved in invoking interface methods are such that they are intrinsically more "sensitive" to bad and/or NIL pointers than regular methods, tho this may have been more specifically a problem with NIL pointers (it is impossible to write a "NIL-safe" interface method afaik, in the same way that you can with a regular, non-virtual method, for example). – Deltics Jan 28 '11 at 03:58
  • 1
    @David addendum :) The bug in TInterfacedObject is that if the object references itself (or causes itself to be referenced) via an interface reference during execution of the destruction chain, then it will double-destroy itself. This is similar to the problem caused if an object references itself (or causes itself to be referenced) by an interface reference during execution of the CONstructor chain (the object will be destroyed before it has finished creating). They fixed the constructor chain issue, but (as of D2010 at least) haven't fixed the destructor chain. – Deltics Jan 28 '11 at 04:00
  • @deltics i'm pretty sure none of my code triggers any of these issues but it's great to learn more and be better armed in case a future change trips over one of these problems. – David Heffernan Jan 28 '11 at 04:05
  • @deltics I still think that you should edit your answer because Re0sless answer mentions automatic destruction via ref count which does not happen here. Also your answer was much improved by your edit after I had provoked a re read so I don't feel too bad over this whole affair! ;-) – David Heffernan Jan 28 '11 at 04:11
  • Yep, you're right - the reference to Re0ssless was a bad idea. Gone. :) – Deltics Jan 28 '11 at 05:11
  • @Deltics: did you report this TInterfacedObject destructor bug in QC? – Jeroen Wiert Pluimers Jan 28 '11 at 11:29
  • Ok, done. Keeping the object reference and freeing it after nilling worked perfectly. Many thanks again to all of you. – Pablo Venturino Jan 28 '11 at 11:47
  • @Deltics How and when was the TInterfacedObject bug fixed? The code for TInterfacedObject is identical between D6 and D2010. Was it fixed before D6? Is the fix in a different class? Is the fix in the way the compiler generates code that uses interface variables. – David Heffernan Jan 28 '11 at 12:01
  • @Deltics @Rob I updated my answer to retract the numerous errors I made. I can now see that I was talking utter rubbish when I talked about interface methods that do not "touch self". Since, for an interface method, the decision of which method to invoke is made by the instance, it is clear that a valid instance must be in place. I cannot believe I could not see that, no excuses. – David Heffernan Jan 28 '11 at 13:55
  • @David - I shall blog about it in full in the next few days, as I think this would be an area of interest for many. :) – Deltics Jan 28 '11 at 21:41
  • @Deltics That would be interesting, but I think I'm fully up to speed now!!! ;-) Seriously though, I trust you didn't take too much offense and that my retractions made good my earlier ill-advised remarks. – David Heffernan Jan 28 '11 at 21:42
  • Absolutely no offense taken whatsoever. :) – Deltics Jan 31 '11 at 07:34
6

You should not use TComponent as base class for your interfaced objects, you should use TInterfacedObject instead. TInerfacedObject has implemented the necessary functions to handle lifetime management for interfaces in Delphi. You should also never mix accessing your interface as interface and object. Here is a modification of your code that works just fine with no memory leaks.

program Project2;
{$APPTYPE CONSOLE}

uses
    SysUtils, Classes;

type
    ISomeInterface = interface
        function SomeFunction: string;
    end;

    TSomeClass = class(TInterfacedObject, ISomeInterface)
    public
        called: Integer;
        function SomeFunction: string;
    end;

var
    SomeInterface: ISomeInterface;
    i: Integer;

function TSomeClass.SomeFunction: string;
begin
    Result := 'SomeFunction called!';
end;

begin
    try
        SomeInterface := nil;
        for i := 1 to 10 do
        begin
            if SomeInterface <> nil then
            begin
                SomeInterface := nil;
            end;
            SomeInterface := TSomeClass.Create;
            SomeInterface.SomeFunction;
        end;
    except
        on e: Exception do
            WriteLn(e.message);
    end;
end.
Mikael Eriksson
  • 128,815
  • 20
  • 186
  • 261
  • I don't agree with "never mix accessing your interface as interface and object". It's fine to do so provided that your object implements `IInterface` without reference counting and lifetime management. And in fact that's how `TComponent` does it. – David Heffernan Jan 27 '11 at 22:17
  • 1
    Yes. Meant never mix lifetime management principles. TXMLDocument is a good example. Use it as an interface (IXMLDocument) or use it as TXMLDocument (with owner). Assign a TXMLDocument to a interface and the call free you will get access violation on IXMLDocument release. – Mikael Eriksson Jan 27 '11 at 22:44
  • Thanks for the tip. Sadly, this is legacy code I'm mantaining and every class is a TComponent, and it's not an option at this point to change that. Still thanks for the answer, I've learned from it. – Pablo Venturino Jan 27 '11 at 23:33
  • @David, even if your class doesn't count references, *you* still have to count them so that you can be certain that no references remain when it's time to call Free on the object. (You don't have to count them in code; instead, you can just tally them in a notepad you keep next to you on your desk and check each time before you compile your project.) Since the compiler is going to insert calls to the reference-counting functions no matter what, I say you may as well let the class keep track of the count for itself. – Rob Kennedy Jan 27 '11 at 23:52
  • @rob that's true even when you aren't using interfaces. It's true for all non garbage collected environments – David Heffernan Jan 28 '11 at 00:02
  • 3
    Kind of, @David. The problem is more pronounced with interfaces. With ordinary object references, you don't actually have to clear all references before freeing the object. You just need to make sure you don't *use* the leftover values of remaining variables. But with interfaces, even if the *programmer* doesn't use a stale interface variable anymore, the *compiler* will use it implicitly when the variable goes out of scope or when it gets re-assigned. Since the problem is triggered by code you can't see, it's easier to forget that it's lurking and causing crashes. – Rob Kennedy Jan 28 '11 at 00:09
4

When you have an interface variable such as your ISomeInterface var, you don't need to free it, as it is reference counted and will free it's self when it drops out of scope.

Read Rob Kennedy's answer to this question: Delphi7, passing object's interface - causes Invalid Pointer Operation when freeing the object

From http://delphi.about.com/od/beginners/l/aa113004a.htm

As soon as the interface goes out of scope, Delphi will actually free the interface for you automatically! Interface's declared within a procedure or function will naturally fall out of scope when the procedure ends. Interface's declared within a class or are declared globally will naturally fall out of scope when the object is freed or the program ends.

If in doubt try using FastMM memory manager and tuning on the memory leak detection, to see if the object is leaked.

Community
  • 1
  • 1
Re0sless
  • 10,022
  • 5
  • 47
  • 64
  • 1
    This is incorrect since `TComponent` does not use reference counting; `AddRef` and `Release` return -1 – David Heffernan Jan 27 '11 at 22:14
  • I don't exactly know the reason, but actually, I added a destructor Destroy to the class and tried removing the reference without freeing it. The destructor was never called. – Pablo Venturino Jan 27 '11 at 23:05
  • @pablo in the posted code the only thing that calls free on your object is your explicit call to free. There is no reference counting on TComponent. – David Heffernan Jan 27 '11 at 23:12
  • I understand this, since many answers have mentioned that TComponent does not keep reference count. I understand that I must free the object myself (I don't know if I'd still have to if I created the TSomeClass with an owner I can later free). – Pablo Venturino Jan 27 '11 at 23:44
  • 1
    If you assign an owner to the `TSomeClass` instance, then the owner will free that object later. You don't need to call `Free` in that case (although it's harmless to call it anyway — the object will notify its owner that it's been destroyed). – Rob Kennedy Jan 28 '11 at 00:12
3

You're mixing it. All depends on _AddRef and _Release methods. Check how TInterfacedObject in system.pas is declared.

Delphi just calls _AddRef and _Release methods during using Interafaces, calling Free depends on how object implements _Relase method.

TComponent is not automatically destroyed (except Com object components).

var
  o: TSomeClass;
begin
  ..
  ..
  begin
    o := SomeInterface.AsObject as TSomeClass;
    SomeInterface := nil;  // now we decrease reference counter, but it will not free anything unless you rewrite _AddRef and _Release methods
    o.Free; // just free object by your own
 end;
DiGi
  • 2,480
  • 18
  • 26
  • +1 You are stating correctly, albeit in a slightly obtuse manner, that `TComponent` implementation of `IInterface` is no reference counted. – David Heffernan Jan 27 '11 at 22:23
  • It's not really that you're decreasing the reference counter, but that you're **ensuring that there are no more references**. If there are any references remaining, then the compiler will attempt to call `_Release` on them (regardless of whether the object was "using" reference counting), and that will fail because the underlying object has already been destroyed. – Rob Kennedy Jan 27 '11 at 23:26
  • @rob this clears up my misunderstanding, thanks. My non ref count addref/release methods don't touch self and are immune. The implementation in TComponent which does touch Self is a ticking time bomb! – David Heffernan Jan 27 '11 at 23:58