Delphi: why doesn't FreeAndNil *really* nil my object?
Asked Answered
C

4

6

I want to pass an object A to a second object B, have B do some processing and finally release A in case it's not needed anymore. A watered down version is given below.

program Project6;
{$APPTYPE CONSOLE}
uses
  SysUtils;
type
  TMyObject = class(TObject)
  public
    FField1:  string;
    FField2:  string;
  end;
  TBigObject = class(TObject)
  public
    FMyObject:  TMyObject;
    procedure Bind(var MyObject:  TMyObject);
    procedure Free();
  end;
procedure TBigObject.Bind(var MyObject: TMyObject);
begin
  FMyObject := MyObject;
end;
procedure TBigObject.Free;
begin
  FreeAndNil(FMyObject);
  Destroy();
end;
var
  MyObject:   TMyObject;
  BigObject:  TBigObject;
begin
  try
    MyObject := TMyObject.Create();
    BigObject := TBigObject.Create();
    BigObject.Bind(MyObject);
    BigObject.Free();
    if (Assigned(MyObject)) then begin
      WriteLn('Set MyObject free!');
      MyObject.Free();
    end;
    ReadLn;
  except
    on E: Exception do
      Writeln(E.ClassName, ': ', E.Message);
  end;
end.

(Never mind the awful design.) Now, what I don't understand is why FreeAndNil actually does free MyObject, yet Assigned(MyObject) is evaluated to true (giving an AV at MyObject.Free()).

Could someone please help enlighten me?

Carpal answered 7/11, 2011 at 12:18 Comment(4)
You should rename your method Free! There is already declared such a method in TObject. It would be far better to override Destroy and call inherited inside.Feed
@Uwe: yeah, that was a poor, poor choice of name. :) I'd rather let it stand as it is, though, due to the numerous comment/answers from yourself and others. Thanks!Carpal
Simple answer: objects can not be niled, only references to objects can :)Mucus
Simpler code example; A := B; B := nil; Note that only B is now nil, not necessarily A.Milfordmilhaud
P
15

The reason is simple, you nil one reference but not the other. Consider this example:

var
  Obj1, Obj2: TObject;
begin
  Obj1 := TObject.Create;
  Obj2 := Obj1;
  FreeAndNil(Obj1);
  // Obj1 is released and nil, Obj2 is non-nil but now points to undefined memory
  // ie. accessing it will cause access violations
end;
Party answered 7/11, 2011 at 12:26 Comment(10)
thank's for the speedy (and correct!) reply, man. In hindsight it's so obvious that I feel rather embarrassed ... deadlines can make you (at lest me) post prematurely, it seems. :)Carpal
Welcome! Don't be embarassed, this happens to everyone. (agent voice) "Never send a human to do a machine's job." ;-)Party
BTW, to be fair, I wasn't the first, @CodeInChaos posted his answer before me.Party
And I usually say, especially to those with problems managing memory: avoid FreeAndNil as much as possible. It is often not necessary, and it often does not what you want it to do. Good code can do without FreeAndNil.Polyzoic
@rudy often that's bad advice. Using FreeAndNil makes it easier to find bugs. Not using it does not.Pincenez
Ah, the old FreeAndNil vs TObject.Free discussion... This thing can go forever ehehehehe :-)Arbitrary
This is not the place to discuss it, and it has been discussed on the Embarcadero forums already but in my experience, the use of FreeAndNil is often (not always!) a sign of bad underlying design.Polyzoic
@RudyVelthuis Can you link that discussion? When I still programmed in delphi I used it for almost all non local variable destructions. But I'm interested in knowing the downsides.Lemon
@CodeInChaos: It has been discussed several times, a few times even with participation of the Delphi developer team. I don't have a link, sorry. It came down to the fact that FreeAndNil is either used out of misunderstanding of the underlying memory issues, or even as a design tool, to avoid AVs or memory errors. If that is necessary, the code should IMO be redesigned. Proper design hardly ever needs it, not even for debugging purposes.Polyzoic
@Lemon The argument amounts to "Some people don't know what it does and think it does more than it really does. So don't anybody use it." Now, I think that if you can't understand what FreeAndNil does, then you need to go right back to basics. It appears to me that the baby has been thrown out with the bath water.Pincenez
L
16

MyObject is a different variable from the field FMyObject. And you're only niling the field FMyObject.

FreeAndNil frees to object pointed to, and nils the variable you passed in. It doesn't magically discover and nil all other variables that point to the object you freed.

FreeAndNil(FMyObject); does the same thing as:

object(FMyObject).Free();
FMyObject=nil;

(Technically this is not entirely correct, the cast to object is a reinterpret cast due to the untyped var parameter, but that's not relevant here)

And that obviously only modifies FMyObject and not MyObject


Oh I just noticed that you're hiding the original Free method? That's insane. FreeAndNil still uses the original Free. That doesn't hit you in your example because you call Free on a variable with the static type TBigObject and not FreeAndNil. But it's a receipt for disaster.

You should instead override the destructor Destroy.

Lemon answered 7/11, 2011 at 12:21 Comment(4)
@CIC: thank you for your explanation (+1). I'll credit TOndrej, though, as he was first.Carpal
@Carpal in fact CodeInChaos was first but you should accept the best rather than the first, whatever you deem to be the best.Pincenez
@David: it seems I got a different time reading than you guys. Yours, CIC and TOndrejs answer are all satisfactory to me, thus I'd accept the first correct answer.Carpal
@Carpal Nope, we've all got the same time. And CIC was first. I think CIC's answer is the best and that's the one I would have accepted.Pincenez
P
15

The reason is simple, you nil one reference but not the other. Consider this example:

var
  Obj1, Obj2: TObject;
begin
  Obj1 := TObject.Create;
  Obj2 := Obj1;
  FreeAndNil(Obj1);
  // Obj1 is released and nil, Obj2 is non-nil but now points to undefined memory
  // ie. accessing it will cause access violations
end;
Party answered 7/11, 2011 at 12:26 Comment(10)
thank's for the speedy (and correct!) reply, man. In hindsight it's so obvious that I feel rather embarrassed ... deadlines can make you (at lest me) post prematurely, it seems. :)Carpal
Welcome! Don't be embarassed, this happens to everyone. (agent voice) "Never send a human to do a machine's job." ;-)Party
BTW, to be fair, I wasn't the first, @CodeInChaos posted his answer before me.Party
And I usually say, especially to those with problems managing memory: avoid FreeAndNil as much as possible. It is often not necessary, and it often does not what you want it to do. Good code can do without FreeAndNil.Polyzoic
@rudy often that's bad advice. Using FreeAndNil makes it easier to find bugs. Not using it does not.Pincenez
Ah, the old FreeAndNil vs TObject.Free discussion... This thing can go forever ehehehehe :-)Arbitrary
This is not the place to discuss it, and it has been discussed on the Embarcadero forums already but in my experience, the use of FreeAndNil is often (not always!) a sign of bad underlying design.Polyzoic
@RudyVelthuis Can you link that discussion? When I still programmed in delphi I used it for almost all non local variable destructions. But I'm interested in knowing the downsides.Lemon
@CodeInChaos: It has been discussed several times, a few times even with participation of the Delphi developer team. I don't have a link, sorry. It came down to the fact that FreeAndNil is either used out of misunderstanding of the underlying memory issues, or even as a design tool, to avoid AVs or memory errors. If that is necessary, the code should IMO be redesigned. Proper design hardly ever needs it, not even for debugging purposes.Polyzoic
@Lemon The argument amounts to "Some people don't know what it does and think it does more than it really does. So don't anybody use it." Now, I think that if you can't understand what FreeAndNil does, then you need to go right back to basics. It appears to me that the baby has been thrown out with the bath water.Pincenez
P
9

You have two copies of the reference to the object but are only setting one of them to nil. Your code is equivalent to this:

i := 1;
j := i;
i := 0;
Writeln(j);//outputs 1

I'm using integers in this example because I'm sure you are familiar with how they work. Object references, which are really just pointers, behave in exactly the same way.

Casting the example in terms of object references makes it look like this:

obj1 := TObject.Create;
obj2 := obj1;
obj1.Free;//these two lines are
obj1 := nil;//equivalent to FreeAndNil
//but obj2 still refers to the destroyed object

Aside: You should never call Destroy directly and never declare a method called Free. Instead override Destroy and call the static Free defined in TObject, or indeed FreeAndNil.

Pincenez answered 7/11, 2011 at 12:23 Comment(4)
thank you for your answer and for your suggestions regarding usage of Free and Destroy.Carpal
@David These two lines are not fully equivalent to FreeAndNil since FreeAndNil nils the reference first and then destroys the object. This does not matter in most cases though. +1 for the good explanationRieger
@smasher yes that's a fair point but I hope you'll forgive me in the aid of a smooth exposition!Pincenez
@DavidHeffernan: very school-style explanation... Like it a lot!! +1Arbitrary
P
2

There are a few peculiarities in your code.

First, you should not re-write Free, you should override the virtual destructor (Destroy) of your class.

But ISTM that BigObject is not the owner of MyObject, so BigObject should not try to free it at all.

As CodeInChaos already said, FreeAndNil only frees one variable, in this case the FMyObject field. FreeAndNil is not required anyway, since nothing can happen after the object was freed.

Assigned can't be used to check if an object was freed already. It can only check for nil, and FreeAndNil only sets one reference to nil, not the object itself (this is impossible).

Your program design should be thus, that an object can and will only be freed if nothing is accessing it anymore.

Polyzoic answered 7/11, 2011 at 13:1 Comment(1)
Thanks Rudy for sharing some insightful thoughts about design.Carpal

© 2022 - 2024 — McMap. All rights reserved.