It seems that my previous post about FreeAndNil sparked a little controversy. Some of you jumped right on board and flat agreed with my assertion. Others took a very defensive approach. Still others, kept an “arms-length” view. Actually, the whole discussion in the comments was very enjoyable to read. There were some very excellent cases on both sides. Whether or not you agreed with my assertion, it was very clear that an example of why I felt the need to make that post was in order.
I wanted to include an example in my first draft of the original post, but I felt that it would come across as too contrived. This time, instead of including some contrived hunk of code that only serves to cloud the issue at hand, I’m going to try a narrative approach and let the reader decide if this is something they need to consider. I may fall flat on my face with this, but I want to try and be as descriptive as I can without the code itself getting in the way. It’s an experiment. Since many of my readers are, presumably, Delphi or C++Builder developers and have some working knowledge of the VCL framework, I will try and present some of the problems and potential solutions in terms of the services that VCL provides.
To start off, the most common case I’ve seen where FreeAndNil can lead to strange behaviors or even memory leaks is when you have a component with a object reference field that is allocated “lazily.” What I mean is that you decide you don’t need burn the memory this object takes up all the time so you leave the field nil and don’t create the instance in the constructor. You rely on the fact that it is nil to know that you need to allocate it. This may seem like the perfect case where you should use FreeAndNil! That is in-fact the very problem. There are cases where you should FreeAndNil in this scenario. The scenario I’m about to describe is not such a case.
If you recall from the previous post, I was specifically referring to using FreeAndNil in the destructor. This is where a very careful dance has to happen. A common scenario in VCL code is to hold references to other component from a given component. Because you are holding a reference there is a built-in mechanism that allows you coordinate the interactions between the components by knowing when a given component is being destroyed. There is the Notification virtual method you can override to know if the component being destroyed is the one to which you have a reference. The general pattern here is to simply nil out your reference.
The problem comes in when you decide that you need to grab some more information out of the object while it is in the throes of destruction. This is where things get dangerous. Just the act of referencing the instance can have dire consequences. Where this can actually cause a memory leak was if the field, property, or method accessed caused the object to lazily allocate that instance I just talked about above. What if the code to destroy that instance was already run in the destructor by the time the Notification method was called? Now you’ve just allocated an instance which has no way to be freed. It’s a leak. It’s also a case where a nil field will never actually cause a crash because you were sooo careful to check for nil and allocate the field if needed. You’ve traded a crash for a memory leak. I’ll let you decide whether or not that is right for your case. My opinion is that leak or crash, it is simply not good design to access an instance that is in the process of being destroyed.
“Oh, I never do that!” That’s probably true, however what about the user’s of your component? Do they understand the internal workings of your component and know that accessing the instance while it is in the throes of destruction is bad? What if it “worked” in v1 of your component and v2 changed some of the internals? Do they even know that the the instance is being destroyed? Luckily, VCL has provided a solution to this by way of the ComponentState. Before the destructor is called that starts the whole destruction process, the virtual method, BeforeDestruction is called which sets the csDestroying flag. This can now be used as a cue for any given component instance whether or not it is being destroyed.
While my post indicting FreeAndNil as not being your friend may have come across as a blanket statement decrying its wanton use, I was clearly not articulating as well as I should that blindly using FreeAndNil without understanding the consequences of its effect on the system as a whole, is likely to bite you. My above example is but one case where you should be very careful about accessing an object in the process of destruction. My point was that using FreeAndNil can sometimes appear to solve the actual problem, when in fact if has merely traded it for another, more insidious, hard to find problem. A problem that doesn’t bite immediately.
Very nice explanation. But it's "in the throes of" with an E.
ReplyDeleteThanks, Mason. Fixed.
ReplyDeleteEffect not affect in the last paragraph.
ReplyDeleteNice post!
Jim, I guess I know people actually read my ramblings :-). Fixed.
ReplyDeleteI've rewritten this post about 10 times now, pointing out the various flaws in what you have suggested here. Rather than a long, tedious(/annoyed) point by point here however, I think my response is best summed up with the following single statement:
ReplyDeleteYou have resorted to VERY flimsy rationalizations here.
Nice post and I agree with your scenario. However, consider this: users of my class may not only reference my inner instances when I am in the throes of destruction. They may still have references to my inner instances around after I am done destroying myself and my inner instances. Something that easily happens whenever you expose an inner instance through a property. For example the Font or Canvas in many controls. Even when it is only readonly, the reference can get out. By not using FreeAndNil on my inner instances, I now am possibly faced with intermittent and hard to debug access violations as not all uses of the freed but not nilled instance will cause one - it will usually only happen when the memory formerly used by my (inner)instance is finally overwritten... By using FreeAndNil, while it may hide double free's and may cause memory leaks in the scenario you described, I at least ensure that anybody having dangling references around after I destroyed myself gets an AV as soon as they use it. For this reason, using FreeAndNil even in destructors actually has proven itself invaluable in exposing problems that would otherwise be very hard to debug. However, reading your previous post did cause an "aha.... d'oh" experience, as it addresses both problems. So, many thanks for that one!
ReplyDelete"hard to find problem"?
ReplyDeleteAny advanced memory manager will show you mem-leak on object that was allocated during destructor call. You'll solve the problem immediately.
On other side: suppose you use Free. Now what? What if there will be no crash for some reason? How are you going to find this bug? What if (for amazing coincidence) code allocates buffer right on the same place as old object was? That is actually very likely - as MM tends to re-use memory (especially if buffer will be approx. the same size as object).
You access the object - you damage your buffer. What if it was a buffer for saving object's user data to file? Oops, you just trashed your results and you have no idea why.
Sure, you may use advanced memory manager with debug mode, but if misuse doesn't overflow any buffer - MM will not report anything.
Still, a very good point, thanks for the post :D
While I understand your example (I think) I still think that the original post was misleading. In retrospect, it would be much better to write the current post first ;).
ReplyDeleteThe problem here is that various people jumped on the "FreeAndNil is bad" bandwagon without (I think) fully understanding why FreeAndNil is good for. And I still think that "Use FreeAndNil" should be a default for every Delphi programmer. Those, who doesn't understand why, would fare much better by using it at all times. The small minority of better informed people would sometimes NOT use it, but then they would know the reason why and would know how to defend the choice.
How about adding a FreeThenNil procedure that we obsessive compulsive nil'ers can use without risking side affects in destructors?
ReplyDelete@Allen
ReplyDeleteit seems to me that while trying to prove that using FreeAndNil everytime means 'bad design' you provide the example that is 'bad design' itself :)
so, if you are even correct, then why to replace one bad design by another? :)
if you don't think about lifetime of your objects of class you are about to write, then why to appeal to FreeAndNil calls?
what i mean here is: first you need to think about class(or code) in general and decide how it will work and just then start implementing. i see no reason why you will not be able to use FreeAndNil in this case. instead you link outside code to your class's private fields (do you call it good design, huh?) and then explain that 'i should not use FreeAndNil here because it may result some problems then'.
i'm sorry, but the design flaw here is in the way you're making your class, but not in place in destructor's code then.
FreeAndNil gives certain benefits for debugging your code. yes, having strict AV may look a bit dirty way of debugging, but it is:
a) simple to understand
b) easy to locate
if work in large projects, the simplicity of overall development process takes one of major requirements, otherwise there is a risk that project will never be completed. this IS important in real life.
what you are talking about is a good explanation, as people mentioned above, however it contains potentially more problems and the worst thing is that they are most likely hard to debug/fix - especially in deployment terms.
PS. of cause, i can think about i'm making mistake here or thinking wrong way. the best way to know that is actually - look at real example. if you have time, pls make simple project that we can compile on our computers. hypothetical thoughts do not look convincing. it's hard to understand why it may be necessary(or OK) to play tricks with pointers in destructor's code. well, i do think that if you need to do that, then that is already a signal about bad design.
@Allen: So, let's get this straight: you are advocating leaving a pointer to dead object un-nil'd during destruction, so that you /might/ get a crash instead of definitely getting a memory leak?
ReplyDeleteThis seems pretty thin. Remember your subject is 'FreeAndNil is your enemy', not 'If you mess with a component with csDestroying set, you deserve whatever you get'.
Or maybe you aren't suggesting this. As you yourself say, it is arguable whether FreeAndNil helps or hinders here... but then that doesn't feel like much support for all this 'enemy' talk.
I think the chap who had the right idea was the blogger who advocated a variation of FreeAndNil - say FreeAndDoom - which set the object reference to a known invalid value (I think he chose the value 1 cast to a pointer). If applied in this case that you describe, the too-late lazy initialisation would be prevented by the non-nil value in the Assigned() test, and the desired A/V would explode at once, in the right place, right by the buggy code.
After having read the two posts and comments, I'd suggest that using FreeAndNil in general will allow you to detect and fix more flaws than if you were not using it. Lazy allocation could be a hard to spot problem, but as already pointed out - that would show up as a leak.
ReplyDeleteAn alternative to FreeAndNil in the destructor, could be to assign a const DestructorPtr = Pointer(Integer(Nil)+1); (or some other magic number) - and make a FreeAndDestruct routine for use in destructors. If you then use an assert in the lazy allocator to check that the lazy pointer is different from DestructorPtr, you would also be able to catch framework misuse or uninteded re-creation of lazily allocated objects.
As a compoment writer, I consider FreeAndNil a very useful resource. Since Delphi does not provide a memory managed environment, que responsibility of taking care of memory is always part of our lives and in such a context, FreeAndNil participates makes things easier and safer. There are situations when you can take less care and situations when your design must be more complex to guarantee a decent behavior of you class and FreeAndNil alone will not solve the matter. However, by my experience, using FreeAndNill is always good practice. The thing is that in some (very rare) cases, FreeAndNil is just *part* of the good solution.
ReplyDeleteThe present case is Very weak and Wrong reference to object on destroy hence wrong design/usuage at first place.
ReplyDelete1. When Notification is invoked with opRemove, never use property way to access the variable -> no memory leak. Must use straight reference to the variable or better yet, use the passed in AComponent parameter itself.
2. Trashing memory is most/more dangerous than a memory leak.
3. You can easily detect memory leak with current tool but you will be extremely difficult to detect/debug with trashing memory or pointer points to a wrong data
Cheers
I'd still like to see an example. Basically, I can only say that when using FreeAndNil the way I use it, I have never had any problem with it. Nor have any people who use my components ever had a problem with FreeAndNil.
ReplyDeleteI tend to comment my object references by "ref-only", and "clean". If delphi syntax supported some kind of decorator, I would probably use it:
FMyObject1:TMyClassA; @(ref); // don't free me from destructor!
FMyObject2:TMyClassB; @(owner); // don't forget to free me in the destructor!
FMyObject3:TMyClassC; @(lazy,owner); // I could be nil! but if I'm not nil, free me in destructor
Anyways, if you could come up with even the most contrived example, I'd be tempted to concede you have more of a point than here. I almost could imagine that this could almost have happened once, maybe twice, in the history of delphi programming. Making it, well, a non-factor.
Even a single anecdote (this actually happened to me! watch out!) carries more weight than a verifiably leaky abstraction.
If you check component state, and are aware of your component and class set-up and tear-down mechanisms, including lazily created objects, this bug will never in a million years be on your radar.
Now, if you guys could find a way to fix Error Insight, and that nasty habit that RAD Studio has of not updating identcache files, leading to bogus error insight spews, and it turned out that this had something to do with FreeAndNil, then I'd give you the point. :-)
W
@Michael,
ReplyDelete"it seems to me that while trying to prove that using FreeAndNil everytime means ‘bad design’ you provide the example that is ‘bad design’ itself"
I'm not saying that at all. My point is that using it as a "fix" for what is possibly a bad design isn't really a "fix" is it?
@Xepol,
"You have resorted to VERY flimsy rationalizations here."
I'm not trying to rationalize anything here. I am trying to merely point out that there are cases where FreeAndNil isn't the right approach. I'm trying to guard against the extremes (always use it vs. never use it) and suggest that a deeper understanding of the system or consideration of the design and architecture is far better than relying on the some "magical" property of FreeAndNil.
Another point I can make here that may seem like heresy is that I sometimes get the feeling that the use of FreeAndNil by some is an unconscious desire to have true garbage collection when it does nothing of the sort ;-). (oh boy, let the sparks fly on that one ;-).
Allen, of course you are trying to rationalize here.
ReplyDeleteYou have created a situation that doesn't NEED FreeAndNil to have exactly the same problem.
You have combined Defered construction of resources and a usage pattern that can ONLY cause problems and blaming the method an object is freed for the problem.
Frankly, FreeAndNil is the only part of your scenario that ISN'T actually a contributing factor to the end result.
Yes, you can do defered creation of resources without FreeAndNil. In the destructor, you could use If Assigned(X) THen X.Free; If someone is using the notification method to do more than nil their reference then you still get the same problem.
That is, unless you were to check for csDestroying in the componentState before initializing the defered resource, and instead raised a "Boy you are dumb" exception instead.
I honestly can't anyone trying to do what you are suggesting because getting a notification that an object is being destroyed puts it in a very questionable state.
FreeAndNil is not the problem in the scenario you present - it is the bad choices that follow.
You have created a scenario that has nothing to do with your core topic (FreeAndNil), and added in a usage pattern that SHOULD cause problems and concluded that FreeAndNil is the cause of the problem.
If is anything other than a poor rationalization, start your weekend now - you need a rest.
@Xepol,
ReplyDeleteYou should think in a larger context. VCL is a service infrastructure. There are clients (components) that depend on that infrastructure. Even worse - they become part of that infrastructure.
Because of the way the infrastructure is designed (yes, the design is debatable; no this is not the point of the topic, let's put it aside), the client depends on some infrastructure internals. The client can either handle problematic situation (aka component destruction) correctly, or not. In the former case, everything works independently of the way the infrastructure is implemented.
In the latter case, though, the infrastructure can either allow the client to proceed even if it is doing wrong or it can crash. Obviously, the best way would be for the infrastructure to explicitly state "the client sucks" but it can't do that. (I definitely don't know enough about all the hoops and loops that the VCL must jump through to work seamlessly with the Win32 API to event start thinking about whether this third option can be implemented or not. Again - a design choice.)
What Allen argues for is that the crash is preferable to the client not knowing about the problem. Actually, I agree with that, but only if we state that the design is fixed and cannot be changed. I'd much rather change the design (but let me repeat again - I'm not saying at all that the design *can* be fixed).
So the bottom line is - if you have a complicated design, it is sometimes better not to nil dead objects. At least that's how I understand the topic. I may be totally wrong.
While I don't know if design can be fixed or not, I do agree with people saying that in that case, invalid reference should be set to TObject(1), not to nil and not to the old (destroyed) object. That way the code would crash always, not just if FastMM happens to be watching over the memory allocation problems.
Consider the case of a multi-threaded app in which a thread can have 0 (zero) or one instance (IOW a singleton).
ReplyDeleteThe thread does by itself an operation and then it dies. So, the best thing for this would be to set its FreeOnTerminate := True;
However because the very last command in the Thread's destructor isn't the FreeAndNil and, as you say, setting the TThread instance to nil elsewhere will have nasty side effects, instead of a simple check for the (in)existence of the above thread (if Assigned(myThread) then...) we need to do other hacks / structures (remember that we are in parallel programming case) in order to see if the thread has really died.
Are you going to fix this?
PS: I don't say that any object when it is freed should have its handle set on nil - just in the case of threads where this thing is more difficult to handle and only in the case in which FreeOnTerminate is set to True - case in which the developer says explicitly 'do your job alone and die'.
@m. Th.
ReplyDeleteIf you ever use FreeOnTerminate, you should never have an external reference to the TThread instance, singleton or otherwise. It is entirely possible to have a "FreeOnTerminate" thread run to completion and be freed before you could even assign the instance to a variable after returning from the constructor.
@Xepol,
ReplyDelete"You have created a scenario that has nothing to do with your core topic (FreeAndNil), and added in a usage pattern that SHOULD cause problems and concluded that FreeAndNil is the cause of the problem."
I've never said that FreeAndNil was the *cause* of the problem at all. I am saying that using FreeAndNil naively to fix a problem the user has already encountered is the problem. FreeAndNil being used before fully understanding the whole system and the implications thereof is the problem. There *are* cases where FreeAndNil is appropriate and is desired. In fact many here have gone to great lengths to point that out and I've done nothing to refute their claims. In fact, I've agreed with nearly all of them.
Yes, my example is somewhat contrived and and can be argued that it is "bad design"... but that was the point. I wanted to show that FreeAndNil wasn't necessarily the "fix", but rather a better evaluation of the entire system to determine if there is a design flaw.
Every framework or system has some subtleties, but using more shotgun approaches to fixing bugs is no substitute to learning the existing system and how it works. Even arguing that the system is designed poorly and that what you're doing *should* be allowed, doesn't change the fact that the system doesn't allow it. I'm reminded of this very thing by a recent post by Raymond Chen: http://blogs.msdn.com/oldnewthing/archive/2010/02/18/9965469.aspx.
Going back to my original point about folks answering questions on Stack Overflow with blanket assertions that they should always use FreeAndNil is doing a disservice to the one asking the question. There are cases where the "golden path" may not be as golden as they've been lead to believe.
???
ReplyDeleteIt is entirely possible to have a "FreeOnTerminate" thread run to completion and be freed before you could even assign the instance to a variable after returning from the constructor.
We all know that all threads are created now in Suspended state, isn't?
We all remember the Fatal Threading Model thread from .non-technical just before releasing the Delphi 2010. In fact it is a long lasting weakpoint which you knew about...
Also, if your statement were true how to assign FreeOnTerminate to an already dangling reference?
So, this code will work in D2010:
var
myThread: TmyThread; //for the sake of our discussion assume that this is global
...
begin
myThread:=nil; //simple sample - ok, it could be done in a better way
if myConditionCheck then
begin
myThread:=TmyThread.Create(True); //'True' is bogus here... - thread is always suspended
myThread.FreeOnTerminate:=True; //'myThread' *is* valid here.
myThread.Start; // ...AND here.
end;
end;
...or perhaps I'm missing something?
@m. Th.,
ReplyDeleteThe point I'm making is that once the thread starts, either by calling Create(False), or explicitly calling Start, if FreeOnTerminate is True, then you should consider any instance variable as potentially invalid.
Specifically, in your example, myThread could become invalid before even returning from the Start method, so accessing the instance once the thread is started, is a race-condition.
@Allen -> "I’ve never said that FreeAndNil was the *cause* of the problem at all." It is the core theme of your scenerio, and check that title.
ReplyDeleteThe fact is that FreeAndNil is the only part of your scenario that has nothing to do with the problem you have constructed here. IT has absolutely nothing to do with it at all. It isn't a magically solution here - it isn't even relevant to the suggested problem is my point.
I agree that FreeAndNil is not a magic bullet. I also agree that you need to understand how things work before you use them.
I do, however, disagree with creating a scenario that has nothing to do with FreeAndNil, showing how it blows up and suggesting it has anything whatsoever to do with FreeAndNil.
Real releavent scenarios are easy to construct. Ie: FreeAndNil can only clear up one pointer, not any copies of those pointers, so the copies are now dangling pointers.
*THAT* would be scenario where FreeAndNil would not be a solution, and could confuse someone unfamiliar with how things work when it did blow up.
Interesting note. It doesn't always blow up. I've noticed that freed memory is not zeroed memory, and you can frequently reference an object once its been freed. As long it does not require any resources that might have been freed when it was destroyed, references can continue to work well past its freeing -until the memory is overwritten with something else (which is a semi-random process).
For THAT reason, I endorse using FreeAndNil everywhere possible - it makes finding dangling pointers somewhat easier.
Unfortunately, you can continue to invoke methods, properties, getters and setters against a nil pointer and the result is usually not an immediate crash, but rather a rogue code crash later. For this reason, I would love to see a flag that tests each and every pointer de-reference - for DEBUGGING only(there are many flags available for that).
so that
P : TMyObject;
Begin
P := Nil;
P.InvokeMethod;
Raises a "Nil Pointer evaluation" exception at P.InvokeMethod instead of seeing how far down a random code path it can run.
And yes, I realize that it is ALSO not a magic bulett, as any copied pointers are still non-nil'ed. But both FreeAndNil and even this suggested compiler behaviour would dramtically help track down the real source of the problem.
And after all, that is the REAL problem in all of this. Not how the object is freed, not what is then done to the pointer - but any flaws in your code design and implementation that might lead you to use that pointer AFTER it is no longer valid.
@Xepol,
ReplyDeleteClearly you're injecting way more into this than I intended. The original premise of what I was trying to explain was that blindly suggesting that using FreeAndNil to fix a problem was naive. Not that FreeAndNil in and of itself was bad.
That "garbage collection" thing has me thinking: Couldn't there be a "not garbage collected" but slightly better memory model than the current Delphi one? I like the Delphi way of doing things a lot better than the C/C++ way of doing things, but there's got to be something that could be done to make it cleaner and easier to manage memory.
ReplyDeleteIsn't there a third way other than adopting (eventually), Oh Please No No no... garbage collection.... Bleah!
W
@Warren,
ReplyDeleteUh.. everything could reference counted... but the cycles would kill you ;-)
I think having a GC would enable whole new programming idioms that would make things more usable. It could also *increase* performance in some scenarios. If we are to attract "new blood", having a GC may help attract some of those fresh-from-college CS guys that learned on managed or dynamic languages more comfortable.
It would also make FreeAndNil moot ;-).
I'm scared as hell of the GC. It's hard enough to find a memory leak in the unmanaged environment. I have no idea how we will do that when GC is used. (And I'm talking about real life bug finding - locating leaks in complicated 24/7 servers that are exhibited only on some deployed installations.)
ReplyDeleteBut that's a whole new topic.
"Specifically, in your example, myThread could become invalid before even returning from the Start method..."
ReplyDeleteExactly!
Exactly this is my request. How can I find that the thread died due of FreeOnTerminate:=True (I'm checking the main thread or from any other thread) if you will not set the myThread to nil as the very last thing in the myThread's destructor?
Having FreeOnTerminate:=True leaves us without any OOTB tooling (like TThread.WaitFor, TThread.Terminated etc.) to check that the thread is destroyed. Or set it to TObject(1). Or to any other deterministic (invalid) value which we can check.
@m.Th. Use OnTerminated and nil your local reference there.
ReplyDelete@Primoz:
ReplyDeleteThanks. I know. But it seems a little bit of kludge. Consider the following scenario:
1. We have the general framework A which deals with file I/O.
2. We have the general framework B which deals with file processing.
In the framework B we have different code (grouped in methods) which are about to execute when different threads from framework A finish. So, we have a 'central place' where we make the links between the two, according to the user's settings via GUI. (eg. thread123.OnTerminate:=FrameworkB.myClass.DoProcessing; etc.)
Of course, framework B, being general, cannot blindly force a 'nil' on the linked thread's reference. So, we need to change the above thing in a more complicated architecture. So, we have a solution for this? Sure. But it would be much more easier if they would add this.
@m.Th.
ReplyDeleteTThread.Create creates and object, stored at some address in memory. Then you store this address in some variable/field and use it in you program. Or you store it in more than one variable/field.
When the TThread object is auto-terminated, it cannot nil this/those variable(s)/field(s) because it has no knowledge of them. This variable/etc is not connected to the TThread object in any way.
What you can do:
type PTThread = ^TThread;
myThread := TMyThread.Create(true);
myThread.NilOnTerminate(@myThread);
myThread.Start;
procedure TMyThread.NilOnTerminate(myThreadAddr: PTThread);
begin
FNilOnTerminate := myThreadAddr;
end;
destructor TMyThread.Destroy;
begin
if assigned(FNilOnTerminate) then
FNilOnTerminate^ := nil;
end;
Hi,
ReplyDeleteI am but a simple man - what I want is
Form1.Create(Self);
Form1.Showmodal;
Form1.Destroy;
Form1:=nil; //bad news
so that somewhere else in the code I can say
if (form1nil) then
begin
stuff..
end;
But How??
should read
ReplyDeleteif not (form1 = nil) then
Steve
I wonder if that be lifted up to the language level.
ReplyDeleteCan methods/properties/fields be marked that they just CAN NOT be accessed during Destroying phase / until Constructing phase completed ?
That way ideally destructor would only call functinos, that are all one afteer anothr guaranteed to not CREATE things and only FREE them
@Marjan Venema:
ReplyDelete> "may still have references to my inner instances around after I am done destroying myself and my inner instances. ... By not using FreeAndNil on my inner instances, I now am possibly faced with intermittent and hard to debug access violations as not all uses of the freed but not nilled instance will cause one ... By using FreeAndNil, ... at least ensure that anybody having dangling references around after I destroyed myself gets an AV as soon as they use it"
I'm afraid you've just highlighted one of the many misconceptions around FreeAndNil. And I suspect this is the sort of thinking that prompted Allen's initial article. I.e. When FreeAndNil is touted as a silver bullet in Stackoverflow articles, people start to rely on it without understanding it's intricacies.
- FreeAndNil sets ONLY the reference passed into the call to nil.
- Objects do NOT hold back-pointers to all references so these can also be set to nil.
- FreeAndNil does not do anything extra to the object's old memory over and above a normal call to Destroy.
- FreeAndNil itself will still AV if it's called on an object that has been destroyed elsewhere, but the reference passed to FreeAndNil is not nil.
So for example:
Obj1 := TObject.Create;
Obj2 := Obj1;
FreeAndNil(Obj1);
Assert(Obj1 = nil);
Assert(Obj2 = nil); //Fails, and any future use of Obj2 may or may not AV depending on what else has happened in the MM.
Marjan, the problem you are concerned about requires Notification registration (built into TComponent).
Basically, any of your clients that hold references to your internal objects, and are concerned they might try use your objects after they've been destroyed should register for notification. Then, as your object enters destruction phase, it notifies all registered observers/subscribers that your object is about to be destroyed. These observers then need to invalidate their references to avoid misusing them.
@Allen
I lay large portion of the blame for misunderstanding of FreeAndNil at shoddy Delphi documentation. It does little to convey real understanding of object creation and destruction and how it relates to the crucial topic of memory management.
For example, the documentation specifically discourages calling TObject.Destroy in favour of TObject.Free, yet the following is perfectly acceptable:
begin
LStrings := TStringList.Create;
try
//
finally
LStrings.Destroy;
//Perfectly safe because LStrings will NOT be nil!
end;
end;
If Delphi documentation touts TObject.Free as the silver bullet to TObject.Destroy; it's little wonder that people consider FreeAndNil to be the silver bullet with garlic frosting. ;)
How ironic that I happened to read this again today. Just a couple of weeks ago I came across a "memory leak" in FMX that was doing this very thing. The property getter would create the object if it did not exist. The class destructor freed the object if it was allocated. But the property was referenced after the object was destroyed, so the getter created another one, which was never freed.
ReplyDeleteHowever, if FreeAndNil had _not_ been used when freeing the reference, the code would have referenced an object that had been destroyed, likely causing an Access Violation or reading memory now allocated to something else. I'll take a memory leak over a memory error any day. But a redesign to correct the issue is preferred. Yes, I added it to QC and I have confidence it was or will be corrected. ;)
http://codeverge.com/embarcadero.delphi.rtl/freeandnil-to-use-or-not-to-use/1067733
ReplyDelete