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.

Reduce development time and get to market faster with RAD Studio, Delphi, or C++Builder. Design. Code. Compile. Deploy.

Start Free Trial   Free Delphi Community Edition   Free C++Builder Community Edition   Upgrade Today  

  • 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. However, 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. ;)
  • @Marjan Venema: > "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. ;)
  • I wonder if that be lifted up to the language level. Can 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
  • should read if not (form1 = nil) then Steve
  • Hi, I 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??
  • @m.Th. TThread.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;
  • @Primoz: Thanks. 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. Use OnTerminated and nil your local reference there.
  • "Specifically, in your example, myThread could become invalid before even returning from the Start method..." Exactly! 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.
  • 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.) But that's a whole new topic.
  • @Warren, Uh.. 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 ;-).
  • 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. Isn't there a third way other than adopting (eventually), Oh Please No No no... garbage collection.... Bleah! W
  • @Xepol, Clearly 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.
  • @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. The 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.