Skip to content

Always use FreeAndNil

Jump to bottom
Michalis Kamburelis edited this page Nov 17, 2023 · 2 revisions

We advise to always (or at least "as much as reasonably possible") free your object instances using FreeAndNil(Obj).

  • Not Obj.Free
  • Not Obj.Destroy
  • Not Obj.Destroy; Obj := nil.

This entire page applies equally to both Delphi or FPC (just as most CGE and Michalis Kamburelis text).

What does Obj.Free do

Obj.Free calls the destructor, only if Obj is not nil.

See https://castle-engine.io/modern_pascal#_how_to_free .

Note that Obj.Free doesn't set Obj to nil. It cannot -- it doesn't even have the address of variable Obj. And it can be used in contexts like Foo.SomeFunctionThatReturnsObject(SomeParam).Free where such variable doesn't exist.

Free implementation is just doing

procedure TObject.Free;
begin
  if Self <> nil then
    Destroy;
end;

Why do we advise FreeAndNil, and what does it do

FreeAndNil(Obj) calls the destructor, only if Obj is not nil, and moreover it makes sure the Obj is set to nil.

I advise to always use FreeAndNil.

Notes:

  • Using FreeAndNil is naturally not a guarantee that all references to this object are niled (for this you need to use free notification, https://castle-engine.io/modern_pascal#_free_notification ) but it's at least a bit better (at least that one reference it set to nil).

  • FreeAndNil also does a trick to set variable to nil before calling the destructor (for this, it saves the reference to a local variable before niling the variable) which is beneficial in some edge-cases (it means the variable is nil already in the middle of the destructor, which is a bit safer and matters in case things get complicated with callbacks or virtual methods that could look at that variable).

  • The extra work done by FreeAndNil shall never impact your speed in practice (both CGE and your user code do thousands of things that affect speed more than additional setting of pointer-sized variable to zero :) ).

But my variable Obj is always a valid pointer, my code already assumes it, why would I bother setting it to nil?

I advise to use FreeAndNil(Obj), not Obj.Free, even if you think the variable should always be a valid pointer.

Because you may be wrong :), I give below examples how one can be easily mistaken about it.

And a nil is better than a dangling pointer:

  • It is always easier to notice in the debugger / Writeln / WritelnLog etc. Any way you look at Obj variable, it will clearly be nil, not 0x1234567.

  • Accessing nil (get/set a field of it, call a virtual method of it) will 100% reliably make access violation at the relevant line.

    In contrast, accessing an invalid (dangling) non-nil pointer is undefined -- maybe it will crash, maybe not (and then obj.MyField may read a memory garbage, setting that field may modify an unpredictable other variable, maybe it was freed + created with exact some pointer (it can often happen in some code) and then it will somewhat behave like it is OK).

Example 1 (how FreeAndNil is better than Free)

destructor TXxx.Destroy;
begin
  Obj.Free;
  SomeOtherFinalizationMethod;
  inherited;
end;

procedure TXxx.SomeOtherFinalizationMethod;
begin
  Obj.MyField.Free;
end;

This above is an incorrect code. But it may be hard to notice. Esp. if you imagine this is the end result of 10 refactors over 10 years :), and the real-life example would have much longer implementations of both Destroy and SomeOtherFinalizationMethod.

The Obj.MyField.Free call above is invalid, it will access Obj.MyField when Obj is an invalid pointer. But maybe it will sometimes work, in case that memory still happens to be accessible and not allocated to something else.

Note:

  • The goal (with using FreeAndNil(Obj)) is not to necessarily force you to do if Obj <> nil then checks before calling all methods of it. I'm not saying you should change SomeOtherFinalizationMethod implementation to check if Obj <> nil then. It is OK to assume that Obj is not nil sometimes. CGE does it too, see https://castle-engine.io/coding_conventions#nil -- we even encourage to assume, by default, that things "are never nil" (and only account for nil when explicitly documented it can happen). It is certainly more readable to be able to assume "Obj is not nil" than to safeguard all code with Obj <> nil checks.

  • The goal is to make it easier to debug, in case your assumption goes wrong. E.g. in above case, if the destructor would do FreeAndNil(Obj), then you would immediately see the problem (100% reliable crash inside SomeOtherFinalizationMethod at line accessing Obj.MyField), and likely conclude "oh, I need to call SomeOtherFinalizationMethod earlier".

Example 2 (how FreeAndNil is better than Free)

Another example with edge-case:

procedure TXxx.Recreate;
begin
  Obj.Free;
  Obj := TSomeObject.Create;
end;

destructor TXxx.Destroy;
begin
  Obj.Free:
  inherited;
end;

The above code is a mistake -- because if TSomeObject constructor will raise an exception, then it will cause (maybe!) another exception from TXxx.Destroy that accesses an invalid pointer at finalization. So you would "mask" one, real, exception (in TSomeObject constructor) with another (in TXxx destructor). When your code crashes, when user submits a bug with a stack trace, you probably wish it will say something like "TSomeObject constructor crashed", not "TXxx.Destroy destructor crashed".

Using FreeAndNil(Obj) always, in both occurrences above instead of Obj.Free, would avoid it. This way there is no "window" inside TXxx.Recreate execution when the Obj is invalid pointer.

Clone this wiki locally