Why FreeAndNil implementation doing Nil before Free?

I can think of two reasons for doing it this way round, neither of which seems at all compelling.

Reason 1: to guarantee that the reference is set to nil in case an exception is raised

The implementation achieves this. If the destructor raises, then the reference is still set to nil. Another way to do so would be with a finally block:

try
  TObject(Obj).Free;
finally
  TObject(Obj) := nil;
end;

The downside of this is performance. Particularly on x86 a try/finally is a little expensive. In such a fundamental routine it is prudent to avoid this expense.

Why do I find the desire to nil at all costs not to be compelling? Well, as soon as destructor start failing you may as well give up. You can no longer reason clearly about your program's state. You cannot tell what failed and what state your program is in. It is my view that the correct course of action in the face of a destructor that raises is to terminate the process.

Reason 2: to ensure that other threads can detect that the object is being destroyed

Again this is achieved but it is of no practical use. Yes you can test whether the reference is assigned or not. But then what? The other thread cannot call methods on the object without synchronization. All you could do is learn whether or not the object is alive. And if that is so, why would it matter if this status changed before or after the destructor runs?

So whilst I present this as a possible reason I cannot believe that anyone in Embarcadero was really swayed by this argument.

There's a variation on David's second reason that is a little more compelling. Although one might argue that if it applies there are other problems that should be fixed.

Reason 3: to ensure event handlers on the same thread can detect that the object is being destroyed

Here's a concocted hypothetical example:

TOwner.HandleCallback;
begin
  if Assigned(FChild) then
    FChild.DoSomething;
end;

TChildClass.Destroy;
begin
  if Assigned(FOnCallback) then FOnCallback;
  inherited Destroy;
end;

Now if TOwner calls:

FChild.Free;
FChild := nil;

FChild will be asked to DoSomething in the middle of its destruction cycle. A certain recipe for disaster. The implementation of FreeAndNil neatly avoids this.

Yes you may argue that firing callback events during destruction is dangerous, but it does have its benefits. There are quite few examples in Delphi/VCL code. Especially if you expand the scope of concern to include calling polymorphic methods - which also put aspects of the destruction sequence outside of your control.

Now I must point out that I'm not aware of any specifc cases in Delphi library code where this problem could manifest. But there are some complex circular dependencies in parts of the VCL. So I wouldn't be surprised if changing the implementation to the more obvious choice in SysUtils leads to a few unpleasant surprises.

The only thing that really bothered me for years is that FreeAndNil(var obj) lacks type safety. I know that due to the lack of adequate language support there was simply no way to do it right, but since we have generics for a while, here's a quick and simple tool that can make your life easier.

type
  TypeSafe = class sealed
  public
    class procedure FreeAndNil<T : class>( var obj : T);  static; inline;
  end;


class procedure TypeSafe.FreeAndNil<T>( var obj : T);
begin
  SysUtils.FreeAndNil( obj);
end;

If you add an overload for the normal FreeAndNil(var Obj) and mark it as deprecatedyou got a fair chance to find all the places where someone hands over an interface pointer to it -- and if the code base is only large enough you will find interesting things, believe me.

procedure FreeAndNil(var Obj); inline; deprecated 'use TypeSafe.FreeAndNil<T>() instead';

Note that you don't even have to specify the <T>since the compiler is smart enough to find out the right type for you. Just add the unit on top and change all

FreeAndNil(foobar);

in your source code into

TypeSafe.FreeAndNil(foobar);

and you're done.