May 23, 2008
When Self in Nil... You Know You are in Trouble
For the non-Delphi programmers out there, self is like this in curly brace languages (C++, C#, Java). It might happen, it you mess things, that yo are debugging a method and realize that self (the object the method is being applied to) is nil, meaning there is no object. Quite often this result in an access violation as you start doing something the data of the the non-existing object.
At times, an object is nil because the constructor that should have returned it failed with an exception (a very spacial case because the object destructor is automatically executed, at least in Delphi). Similarly, an object can be nil because the function that should have returned the reference to the object failed with an exception. This was my case. In a very complex piece of code (with 5 levels of nested try(finally/except blocks) I ended up with code like:
try
obj := GetObjectFromPool (...);
obj.DoThisAndThat;
finally
obj.ReleaseObject;
end;
Now this is trivially wrong, but those statements are 70 lines of code apart, not really easy to spot. Also, in theory GetObjectFromPool should never fail, or at least this is far from common. Also, I cannot really move the GetObjectFromPool call outside of the finally block (again, the code is quite convoluted), so I had to resort to a far-from-clean:
finally
if Assigned (obj)
obj.ReleaseObject;
10 Comments
When Self in Nil... You Know You are in Trouble
Maybe I am missing something here, but my understanding of try..finally blocks is that the proper usage is: obj := GetObjectFromPool(); try obj.DoWhatEver() finally obj.Free; end; This ensures that obj is not nil, and relies on the default constructor mechanism to free the object if an exception is raised in the constructor which is highly unlikely. If such an exception occurs it is likely due memory constraints and as such should be handled by the Application exception handler. Although your solution works, I have seen code added to the top of the try block before obj is created, that produces an exception which then yields an AV in the finally block since obj does not exist. That kind of coding error is much more likely when creating the object within the try block instead of outside as the first line before the try. It is of course, the kind of mistake only a developer that doesn't understand exception handling makes, but I have seen lots of it. I have found using the above code structure and a seperate try..finally for each resource has saved me tons of time in tracking down obscure errors. As such fixing try..finally blocks and/or introducing them is usually one of the first tasks I perform when starting a new maintenance project.Comment by Larry Hengen [larry@NOSPAMhengenconsulting.com] on May 23, 20:08
When Self in Nil... You Know You are in Trouble
I've actually seen self = nil a few times, especially in multi-threaded code. I have a couple methods that start with if not Assigned(self) then Exit; That prevents the method from executing if it is already freed.Comment by Jim McKeeth [http://www.DavinciUnltd.com] on May 23, 20:44
When Self in Nil... You Know You are in Trouble
This of course should be: obj := GetObjectFromPool (...); if assigned(obj) then try obj.DoThisAndThat; finally obj.ReleaseObject; end; I'm sure all three pending comments are containing similar code :)Comment by gabr [http://thedelphigeek.com] on May 23, 21:51
When Self in Nil... You Know You are in Trouble
"far-from-clean" ? Why? I've gotten into the habit of testing proper assignment before trying to free stuff... :)Comment by Fernando Madruga [http://memyselfanddelphi.blogspot.com] on May 23, 23:24
When Self in Nil... You Know You are in Trouble
I know the stardard way of coding an allocate-try-use-finally-release block, thanks for your suggestions. But this was within a very complex piece of code, having many different paths of execution. There are about 150 lines in the method, with up to 5 nested try blocks. The lines shows in the snippet are far apart, and there is exception handling code that in most cases repairs the problem creating another object. As Jim points out, there are circumstances in which you really don't know if you have a valid object or not, so you test before every call or wihint the method itself. not a standard practice, but precious when needed.Comment by Marco Cantù [http://www.marcocantu.com] on May 24, 01:26
When Self in Nil... You Know You are in Trouble
If you can't afford to refactor this monstrum, then I'd do one of those: a) Either test the 'obj' immediately after it is retrieved from the pool and exit if it is not allocated (or throw exception, whichever more appropriate), or b) Test if 'obj' is assigned each time before it is used.Comment by gabr [http://thedelphigeek.com] on May 24, 08:59
When Self in Nil... You Know You are in Trouble
Which is precisely why Free is coded the way it is (and why Free is non-virtual and why you [should] never call Destroy [virtual] directly but always call Free). Same could easily be made to apply here. ReleaseObject could/should be NIL safe in the same way that Free is, and invoke a virtual Release mechanism only if self is not NIL. As for the snippet being "trivially" wrong, absolutely not. It was BADLY wrong, plain and simple, no if's no buts, no qualification. Your "finally" ensured that some method was called on an object for which no reference was assured. You imposed a guarantee on a non-guaranteeable outcome. The correct solution (without addressing the NIL safety of your release mechanism) should be to address the breach of contract between Get/Release and the try/finally, not to apply a Band-Aid to the incorrect (as coded) finally clause. imhoComment by Jolyon Smith [http://www.deltics.co.nz] on May 26, 00:15
When Self in Nil... You Know You are in Trouble
Another possible approach: Free checks for Self being nil. Could you code ReleaseObject to do the same thing? It is presumably a "free"-ish sort of function.Comment by David M on May 26, 03:06
The snippit is not wrong . . . . maybe
We cannot assume the snippit is wrong because we cannot know the value of obj before we entered the try finally block. If we initialize it to nil then the block is completely legal. obj := nil; try obj := GetObjectFromPool (...); obj.DoThisAndThat; finally obj.ReleaseObject; end; Assuming ReleaseObject is non-virtual and it checks its own self reference internally. Much in the same way that Free works.Comment by Jim McKeeth [http://www.DavinciUnltd.com] on June 10, 23:25
Post Your Comment
Click here for posting your feedback to this blog.
There are currently 0 pending (unapproved) messages.



When Self in Nil... You Know You are in Trouble
I think I'd rather do it this way, myself: obj := GetObjectFromPool; if Assigned(obj) then try obj.DoThisAndThat; finally obj.ReleaseObject; end;Comment by Jay Faubion on May 23, 19:59