Allen Bauer, Embarcadero Chief Scientist, wrote an interesting blog entry -
A Case Against FreeAndNIL.
Though I agree with the general gist of that article
(when fixing a defect, don't fix the symptoms, but find out what the root cause of the problem is and fix that),
I don't agree at all that the problem here is in FreeAndNIL usage.
IMO, FreeAndNIL can help in tracking defects down and should be considered a mandatory coding guideline.
I would go as far as to say that not setting a variable to NIL after TObject.Free in the generated code
should be considered a compiler (design) error.
I remember a time when Delphi (and Turbo / Borland Pascal)
compilers didn't generate any warnings for uninitialized local variables.
And such uninitialized variables were often the root cause of many difficult to find defects
(such defects may be quite hard to reproduce,
the behavior of the code referencing such variables depends strongly on the data left on the stack).
These warnings are just indispensable - IME, they save lot's of development time.
All fields in classes are initialized to 0's on instantiation.
Thanks to that, at least these fields don't suffer the problem of not being initialized;
it would be pretty hard (well, impossible in the general case) for the compiler
to generate such warnings for object fields
(across all methods, including inherited methods, and references to fields in external code).
But, just as the case is with local variables,
it would be very easy for the developer to unintentionally reference uninitialized fields.
And now we want take a step back and get rid of all this for pointers to objects?
var
LObject: TSomeObject;
. . .
begin
LObject := TSomeObject.Create;
try
. . .
finally
LObject.Free;
end;
. . .
end;
In the code above, LObject after the finally block is essentially uninitialized -
it points to some location in memory that currently is not being used.
Referencing that memory may cause an error, but it may also continue to work
(depending on what will be in that memory and how we're using the object -
we're back to square one with an uninitialized variable).
It's not enough to have a debugging memory manager that overwrites freed memory
and detects such memory on subsequent tries to reference it.
Again, that memory may already be in use by other instances (and seem valid).
With the new memory manager this is even easier -
a new instance of TSomeObject has a high chance of being placed
at the same memory address as the just freed instance (because the instance size is the same);
and then, references through LObject (although it's freed) will continue to work.
Obviously, this may lead to some really hard to track down problems:
type
TSomeObject = class(TObject)
SomeValue: integer;
end;
. . .
var
LObject1, LObject2: TSomeObject;
begin
LObject1 := TSomeObject.Create;
try
. . .
finally
LObject1.Free;
end;
LObject2 := TSomeObject.Create;
try
Memo.Text := '1: LObject1.SomeValue = ' + IntToStr(LObject1.SomeValue);
LObject2.SomeValue := 5;
Memo.Lines.Add('2: LObject1.SomeValue = ' + IntToStr(LObject1.SomeValue));
Memo.Lines.Add('3: LObject2.SomeValue = ' + IntToStr(LObject2.SomeValue));
LObject1.SomeValue := 10;
Memo.Lines.Add('4: LObject1.SomeValue = ' + IntToStr(LObject1.SomeValue));
Memo.Lines.Add('5: LObject2.SomeValue = ' + IntToStr(LObject2.SomeValue));
finally
LObject2.Free;
end;
end;
With the above code, you get no warnings and no (run-time) errors.
Also, a debugging version of the memory manager will not report any problems either.
But you get the following output:
1: LObject1.SomeValue = 0
2: LObject1.SomeValue = 5
3: LObject2.SomeValue = 5
4: LObject1.SomeValue = 10
5: LObject2.SomeValue = 10
Who would consider this correct program operation?
If you change LObject1.Free to FreeAndNIL(LObject1),
you get a nice run-time error at the first reference to LObject1.SomeValue after LObject1 is freed.
Obviously, this is a contrived example, but... real world scenarios may be much harder to analyze / fix / etc.
Thus, I would argue that:
-
FreeAndNIL is a tool to find defects and not hide them;
a reference to a deallocated variable will always cause a run-time error;
and that's not the case after a call to .Free only.
-
FreeAndNIL should be used in place of .Free whenever possible;
if we're freeing properties, FreeAndNIL cannot be used (it takes a var parameter),
but then .Free and a NIL assignment should be used instead.
Obviously, you may have several pointers to a single instantiated object;
however, it's not an argument against using FreeAndNIL.
Just as you would set those other references to NIL after an object is freed,
you should do the same for the main reference.
Finally, Mr. Bauer argues that after changing .Free to FreeAndNIL,
code becomes sprinkled with unnecessary calls to the Assigned function (that tests whether a variable is NIL).
Now that might be a bad decision -
if we're hiding a defect by not executing code when the reference is NIL, but shouldn't be.
In this case, you need to remove the Assigned call (or a test against NIL)
and get down to the root cause of the problem.
Top
|
Texts
