A recurring subject when it comes to freeing objects and preventing is whether you should just .Free them, thus leaving a invalid reference that should however never be used anymore when the code design is correct, or if you should defensively FreeAndNil() them, thus leaving a nil value that will hopefully trigger AVs more often on improper usage after release.
Allen Bauer recently brought this subject in his blog “A case against FreeAndNil“, arguing that there are better tools than FreeAndNil to diagnose improper usage after release, and that it can hide other issues and lead to other magic bullet solutions, which only further the problem. This is true, and FastMM debug mode can do wonders here, however, quite often, you don’t want to rely on a debug and diagnostic machinery that needs to be switched ON for problems to be detected early on.
Well, if you’re using FreeAndNil() for defensive purposes, don’t abuse it anymore, invest in a few lines of code for a shiny new FreeAndInvalidate():
procedure FreeAndInvalidate(var obj); var temp : TObject; begin temp := TObject(obj); Pointer(obj) := Pointer(1); temp.Free; end;
This function frees the object and sets the reference to an invalid magic value, which will trigger and AV on improper field or virtual method access after release (just like FreeAndNil), but unlike FreeAndNil, it will also AV on multiple .Free attempt, and will not be stopped by “if Assigned()” checks. If you wish even more defense, you can also “sabotage” the VMT pointer of the freed object instance.
With a FreeAndInvalidate() added to your bag of tricks, you can now reserve FreeAndNil usage to situations where having a nil reference is truly part of the design, and no longer abuse it for defensive programming. Of course this is still no magic-bullet, but it’s cheap enough that you can use it in release builds (unlike debug and diagnostic tools), and as a bonus, it makes it obvious when reading the code that the object reference is supposed to be invalid after the call.
7 thoughts on “Don’t abuse FreeAndNil anymore”
I think moving from memory address 0 (nil) to 1 is only moving the problem to another memory address. The next step people will do is to create an AssignedInvalid function (people tend to be pragmatic) that checks for such an invalid object. And history repeats itself.
It’s no magic bullet, and they could do it (and likely will), sure, but then it can be pointed to as an explicit design error, while using FreeAndNil / Assigned aren’t by themselves explicit errors.
Nil is a “valid” value for an object reference, and with Assigned(), there are valid design and usage case scenarios. There exists no such scenario for an AssignedInvalid function, so you can shoot on sight whoever comes up with one, something you can’t do for FreeAndNil/Assigned.
It’s all about moving the issue from a gray area, to one of black and white.
Sorry, but this conflates two problems and discards a perfectly valid strategy simply because it only addresses ONE of those problems.
FreeAndNIL() isn’t always about NIL’ing a reference to an object and hoping that an AV will result if that reference is subsequently re-used. NIL’ing the reference can also be valid in it’s own right.
In a destructor, a partially destructed object may conceivably be re-destroyed. Any objects Free’d during execution of the previous partial destructor chain obviously should not be re-freed.
Yes, a partially destroyed object indicates a potential problem in that class or it’s design, but still may be a valid/unavoidable scenario.
More realistically there may be “instantiate on demand” references that are periodically disposed of, but not replaced until needed. NIL’ing those references when disposed creates a testable condition to trigger the creation of a new object if subsequently referenced.
This sort of thinking process coming out of Embarcadero is illuminating. It seems to indicate to me that the guys “running the show” behind Delphi these days are less concerned about practical work and getting a job of work done, and more interested in abstract language and coding theory and practises.
| thus leaving a invalid reference that should however never be used
| anymore when the code design is correct
I would say: “thus leaving an invalid reference that CAN however never be used anymore.” This does not require FreeAndNil, nor does it need anything like FreeAndInvalidate, IMO.
>NIL’ing the reference can also be valid in it’s own right.
Indeed, this why there is a need to distinguish between valid scenarios and abuses.
>In a destructor, a partially destructed object may conceivably be re-destroyed.
I strongly disagree there: yes, some object reference can be FreeAndNil’ed multiple times during as part of their design during an owner context lifetime, but that should never happen during a destruction chain. If it does happen, then you’re facing either redundant or spaghetti code, with poor maintainability and higher risk of divergence.
Separating FreeAndNil and FreeAndInvalidate allows to make that distinction between a normal nil reference situation, and an abnormal one.
> Any objects Free’d during execution of the previous partial destructor chain obviously should not be re-freed.
Indeed, but FreeAndNil is not a protection against that: a field will stay nil ONLY if the memory of the context that hosted the reference isn’t reallocated during the destruction chain, something you just can’t guarantee in a destruction chain (especially in multi-threading situations).
This can (and does) lead to code initially “working” in single-threaded situations, that very randomly goes bonkers in multi-threading situations, or after some allocations get mixed in the destruction chain …like allocations arising from debug code added to diagnose the very random bonkers situations just mentioned!
>More realistically there may be “instantiate on demand” references
In that case use FreeAndNil, that’s what it’s for.
For all the other situations (IME the vast majority of cases), there is no need to abuse FreeAndNil, and one should never forget that FreeAndNil/FreeAndInvalidate are nothing more than optimistic defensive strategies, so fix the design, that saves pains down the road. 🙂
Alternatively something we do is hook FreeInstance to change the v-table pointer on the heapobject just before the FreeMemory call inside it.
This causes all virtual method calls on the now freed object to throw a CalledAMethodOnAFreedObject exception when the memory hasn’t yet been reused or pageprotected.
The overhead is neglectible and works without replacing .Free or FreeAndNil calls.
I had the same problem when I used freeandnil after close a window.I will string list from a dialog and set them to a listview and then I freeandnil this dialog instance. sometimes, the first row in listview will lose some text.
It is fuck ….. I replace freeandnil with free, the problem disappeared
Comments are closed.