Why should I use Free and not FreeAndNil in a destructor?

Question

I have read A case against FreeAndNil but still don't understand why I cannot use this method in a class destructor ? Can anyone explain.

Update: I think the comment from Eric Grange was most useful for me. The link show that this is not obvious how to deal with it and it is mainly a matter of taste. Also the method FreeAndInvalidate was useful.

Answer 1

The problem with that is that many seem to use FreeAndNil as some magic bullet that will slay that mysterious crash dragon. If using FreeAndNil() in the destructor seems to solve a crash or other memory corruption problems, then you should be digging deeper into the real cause. When I see this, the first question I ask is, why is the instance field being accessed after that instance was destroyed? That typically points to a design problem.

It argues that it hides the real problem you have. It must mean your code is accessing properties/fields/methods of an object that is already destroyed (the destructor is called). So instead of hiding the real problem with FreeAndNil you should really be solving the underlying problem.

This code below would not crash if you would use FreeAndNil PropertyA in the destructor of SomeObject. But it hides the real problem that SomeObject is used after it is destroyed. It is better to solve this design problem (accessing destroyed objects) instead of hiding it.

SomeObject.Free;  // Destructor called
if Assigned(SomeObject.PropertyA) then  // SomeObject is destroyed, but still used
  SomeObject.PropertyA.Method1;  

EDIT

On the other case, one could argue that if FreeAndNil is not used, the code would not crash either. Since even though the object is destroyed, the memory might not be reused and all structures might be in tact. The code above might even run without problems if Free is used to destroy PropertyA instead of FreeAndNil.

And if FreeAndNil was used to destroy SomeObject, you would also see the real problem no matter what the code in the destructor is.

So although I agree with the argument that it could hide the real design flaw and personally do not use FreeAndNil in destructors, it is not some magic bullet to discover such design flaws.

Answer 2

The issue is fairly easy to explain, and the contention around this issue is more subjective than objective. The use of FreeAndNil is simply unnecessary if the variable reference to the object being freed will go out of scope:

procedure Test;
var
  LObj: TObject;
begin
  LObj := TObject.Create;
  try
    {...Do work...}
  finally
    //The LObj variable is going out of scope here,
    // so don't bother nilling it.  No other code can access LObj.

    //FreeAndNil(LObj);
    LObj.Free;
  end; 
end;

In the above code snippet, nilling the LObj variable would be pointless, for the reason given. However, if an object variable can be instantiated and freed several times during the lifetime of an app, then it becomes necessary to check whether the object is indeed instantiated or not. The easy way to check this is whether the object reference has been set to nil. In order to facilitate that setting to nil, the FreeAndNil() method will both free the resources, and set nil for you. Then, in code you can check to see whether the object is instantiated with either LObj = nil or Assigned(LObj).

The case of whether to use .Free or FreeAndNil() in object destructors is a grey area, but for the most part, .Free should be safe, and nilling the references to sub-objects in the destructor should be unnecessary. There are various arguments around how to deal with exceptions in constructors and destructors.

Now pay attention: if you prefer to pick and choose whether to use .Free or FreeAndNil() depending on the specific circumstances outlined above, that's fine, but note that the cost of a bug due to not nilling a freed object reference that is subsequently accessed can be very high. If the pointer is subsequently accessed (object freed but reference not set to nil), it can happen that you are unlucky and the detection of memory corruption occurs many lines of code away from the access to the freed-but-unnilled object reference. This kind of bug can take a very long time to fix, and yes, I know how to use FastMM.

Therefore for some people, including me, it has become habit (a lazy one, perhaps) to simply nil all object pointers when they're freed, even when the nilling is not strictly necessary.