1

Is there a pratical reason why FreeAndNil sets the reference to nil before freeing the object?

procedure FreeAndNil(var Obj);
var
  Temp: TObject;
begin
  Temp := TObject(Obj);
  Pointer(Obj) := nil;
  Temp.Free;
end;

Based on the name of the function, I was expecting something like this:

procedure FreeAndNil(var Obj);
begin
  TObject(Obj).Free;
  Pointer(Obj) := nil;
end;
Fabrizio
  • 7,603
  • 6
  • 44
  • 104
  • 4
    Destructors MUST not raise an exception, but technically, they can, and quite often they do in poorly written code. And if the destructor raises an exception, it will prevent the pointer from being set to `nil` in your second code snippet. And that's bad. You don't want a [dangling pointer](https://en.wikipedia.org/wiki/Dangling_pointer) to a semi-destroyed object. Although the situation is bad in any case (because a destructor raised an exception!), having that dangling pointer makes it much worse in practice. The top snippet, on the other hand, makes sure the pointer is always set to `nil`. – Andreas Rejbrand Jun 19 '20 at 08:14
  • 1
    To be perfectly honest, if a destructor raises an exception, then it would be reasonable to terminate the process. – David Heffernan Jun 19 '20 at 08:56
  • IMO setting the pointer should be using an AtomicExchange call, which makes FreeAnNil threadSafe. So if multiple threads call FreeAndNil on the same variable, only 1 thread will actually free the object. The current FreeAndNil implementation implements a "race condition". – H.Hasenack Jun 19 '20 at 09:17
  • Check out and vote for these related issues: https://quality.embarcadero.com/browse/RSP-29714 and https://quality.embarcadero.com/browse/RSP-29716 – H.Hasenack Jun 19 '20 at 11:11
  • 2
    @H.Hasenack Not a good idea. Objects have owners. Owners can destroy them. There is simply no requirement for what you suggest. – David Heffernan Jun 19 '20 at 11:11
  • @David Hefferman. While this is true, In my application cannot predict what thread is going to destroy the object (or its owner), hence adding the tread safety makes sense for me. I thing the performance penalty is minor to none. – H.Hasenack Jun 19 '20 at 11:18
  • @H.Hasenack then you should rethink your design. Glad I don't have to debug your code. – JensG Jun 19 '20 at 12:22
  • 2
    @H.Hasenack First, there are valid use cases for FreeAndNil. That does not mean it is not being widely abused. Second, thread safety is not something that can be easily achieved by slapping atomic operations on some piece of code. Thread safety usually requires making code thread safe in context larger than simple FreeAndNil operation. Adding "thread-safety" to FreeAndNil would only induce performance penalty for everyone using it without blankly resolving thread issues any particular code might have. Also it is not some extremely complex function that cannot be custom made. – Dalija Prasnikar Jun 19 '20 at 12:38
  • "*if code is well designed, you dont need FreeAndNil*" -- strongly disagree. It is a tool, and one can make great thinghs with tools and one can do poor or bad stuff. The tool is not to blame. But DavidH is 100% right with what he said about owners etc. – JensG Jun 19 '20 at 12:39
  • @Dalija Prasnikar If you take a look at my measurements you'll see performance penalty is minor. But it is there. – H.Hasenack Jun 19 '20 at 12:46
  • @JensG I just bounced your Pun a bit. I totally agree, it is very easy (I proved) to create my own threadsafe FreeAndNil. So as it obviously is a problem to many others, this change should probably not be made. I'll add a reference to this discussion to the issue and let it simmer for a while :) – H.Hasenack Jun 19 '20 at 12:46
  • @H.Hasenack I think you are missing the point... Thread safety does not come from within FreeAndNil. If you need thread safe FreeAndNil that means one of your threads might nuke the object while others are using it. KABOOM!!! Creating thread safe FreeAndNil would be exercise in futility. – Dalija Prasnikar Jun 19 '20 at 12:57
  • 1
    @H.Hasenack If you have multiple threads that operate on the object at the same time with no concern for races, then making `FreeAndNil` threadsafe merely scratches the surface of your problems. – David Heffernan Jun 19 '20 at 13:05
  • I think I am getting convinced. I do not have trouble with FreeAndNil race conditions, obviously my code isnt that bad. It just seemed logical to me that it would be atomic. If it isnt needed, then it isnt needed. – H.Hasenack Jun 19 '20 at 13:12
  • @JensG Can you comment https://quality.embarcadero.com/browse/RSP-29716 as well? – H.Hasenack Jun 19 '20 at 13:24
  • @Dalija Prasnikar Can you comment https://quality.embarcadero.com/browse/RSP-29716 as well? Much appreciated – H.Hasenack Jun 19 '20 at 13:24

0 Answers0