Delphi FreeAndNil: Looking for an alternate implementation
Asked Answered
I

2

4

NOTE: Bear with me, I feel a little "flame grilled" due to some discussions over here and here and some issues I reported here and here.

Some background

Ye olde (pre 10.4) FreeAndNil looked like this:

FreeAndNil(var SomeObject)

The new and fresh FreeAndNil looks like this:

FreeAndNil(const [ref] SomeObject: TObject);

IMO both have their downsides:

  • The old one doesn't do any type checking, so calling FreeAndNil on pointers, records and interfaces compiles just fine, but produces interesting but usually unwanted effects during runtime. (Goes completely berserk or if you are lucky it halts with EAccessViolation, EInvalidOperation etc.)
  • The new one accepts a const parameter, and therefore any object. But then the provided object pointer is actually changed using some hacky-wacky code.
  • You can now call the new FreeAndNil like this: FreeAndNil(TObject.Create) and it will compile and even run just fine. I liked the old FreeAndNil that warned me when I went wrong and provided e.g. a property instead of a field. Unsure what happens if you provide a object type property to this FreeAndNil implementation. Didn't try.

If we would change the signature into FreeAndNil(var SomeObject:TObject) then it will not allow us to pass any other variable type then exactly the TObject type. Which also makes sense, as if it weren't FreeAndNil, one could easily change a variable provided as type TComponent in the routine change the var variable into an object of a completely different type, e.g. TCollection. Of course FreeAndNil will do no such thing, as it always changes the var parameter to nil.

So this makes FreeAndNil a special case. Maybe even special enough to convince delphi to add a compiler magic FreeAndNil implementation? Votes anyone?

Potential work-around

I came up with the code below as an alternative (here as a helper method, but could as well be part of TObject implementation) which kind-a combines both worlds. The Assert will help finding invalid calls during runtime.

procedure TSGObjectHelper.FreeAndNilObj(var aObject);
begin
  if Assigned(self) then
  begin
    Assert(TObject(aObject)=self,ClassName+'.FreeAndNil Wrong parameter provided!');
    pointer(aObject):=nil;
    Destroy;
  end;
end;

Usage would be something like this:

var MyObj:=TSOmeObject.Create;
...
MyObj.FreeAndNilObj(MyObj);

I have actually tested this routine, and it even is slightly faster than the 10.4 FreeAndNil implementation. I guess because I do the assignment check first and call Destroy directly. What I do not like so much is that:

  • the type checking takes place during runtime, and then only if Assertions are ON.
  • it feels like having to pass the same variable twice. Which isn't necessarily true/required. It has to be the same object, and the parameter has to be a variable.

Another investigation

But wouldn't it be great if one could call without the parameter

var MyObj:=TSomeObject.Create;
...
MyObj.FreeAndNil;

So I messed around with the self pointer and managed to set it to nil using the same Hacky-Wacky code that 10.4 utilizes in their FreeAndNil. Well... that worked inside the method, self pointed to nil. But after calling FreeAndNil like this, the MyObj variable wasn't nil, but a stale pointer. (This was what I expected.) Moreover, MyObj could be a property or (the result of) a routine, constructor etc.

so nope over here as well...

And finally the question:

Can you think of a cleaner/better solution or trick that would:

  • FreeAndNil(var aObject:TObject) with not-so-strict type checking compile time (maybe a Compiler directive?) so it allows compiling and calling for variables of any object type.
  • Complains compile time when something is passed that is not a variable/field of some object type
  • Help describing what is the best solution/requirement in RSP-29716
Intangible answered 19/6, 2020 at 22:3 Comment(15)
Just don't use FreeAndNil in the first place! If you really must set the variable to nil, do it yourself.Tupiguarani
I don't want to start a big discussion, but then, @UweRaabe, you lose the "exception safety" property of FreeAndNil.Caroylncarp
@H.Hasenack: You could use a generic FreeAndNil with the class constraint, but it is a bit too verbose: TFreer<TFrog>.FreeAndNil(MyFrog);.Caroylncarp
Huh? FreeAndNill that takes a const [ref], yet it still sets the passed in reference to nil? The function is lying now. While it was rather unsafe previously (the new one is no more safe), at least it didn't lie about it.Wieche
@AllenBauer any chance you would ever consider going back to Embarcadero and cleanup/fix things that have gone awry in your absence?Noahnoak
@AllenBauer Delphi had many great developers working on it, but if I could point single greatest loss for Delphi after Anders Hejlsberg left, it would be your departure. I fully agree with Remy... things have gone awry and there is a lot to fix... I don't expect that happening... but having you back would be great beyond description.Rothman
The only meaningful solution for FreeAndNil would be implementing quality.embarcadero.com/browse/RSP-13724 Overthinking about other solutions will be just waste of time. RSP-29716 is not possible because you could end up with incompatible object instance in your variable - "pass the dog, get the cat" situation.Rothman
Implementing RSP-29716 is impossible because a var parameter (one that goes in and out) is simply invariant - read up about type variance. The trick of treating the value as covariant like FreeAndNil does just works because the out value will only ever be nil and nothing else.Conant
I had a workaround for the FreeAndNil<T>(var aObjectLT) by creating a TObjectHelper=class helprt for TObject. That works, but the restriction of only 1 helper class at any spot makes me unhappy. FTM Embarcadero could add a class method FreeAndNil<T> at TObject level.Intangible
What I meant by a magic compiler routine could be something like an overloaded Dispose, This would take the object variable, and since it is overloaded will link to the object based variant of dispose, and set the provided object pointer to nil. and complain compiletime it its not an object variable. The handling of pointers would not have to change,..Intangible
@Stefan Glienke - this is what I meant with less strict type checking, comparable to the $T+ and $V+ compiler directives. One could apply this then for thisrtoutine (and maybe some others - who knows. But generally it should be a nonono).Intangible
@Allen Bauer - yes! it seems one devil has been traded in for another. Now we get type checking, but lack checking that is a variable etc... and the procedure lies.Intangible
@Intangible What type checking? It's still an untyped const [ref]. I don't understand the reasoning behind the change. FreeAndNill was intended as a convenience for freeing and object and setting the variable to nil. Setting a constant reference to nil is, IMO, a bit hideous. As Dalija has stated, the best and safest solution is one that Delphi doesn't currently support.Wieche
@RemyLebeau, I'm afraid they would not be able to afford me :). All kidding aside, I'm still doing well at Google. I'm learning a lot of new things while also leveraging a lot of my past experience with designing and implementing frameworks and libraries.Wieche
@Allen Bauer I meant to have "relaxed" type checking (comparable to $T- $V- options but for objects and class inheritance) in place when it is declared as FreeAndNil(var aObject:TObject) - Sorry I got a bit lost in all threads and discussions :oIntangible
I
2

Because we cannot create a global procedure FreeAndNil<T:class>(var aObject:T) I would suggest the code below as a method to the TObject class. (rtl change to be made by embarcadero, but does not need a compiler change)

class procedure TObject.InternalFreeAndNil(var Object:TObject); static; // strict private class method
begin
  if Assigned(Object) then
  begin
    var tmp:=Object;
    Object:=nil;
    tmp.Destroy;
  end;
end;


class procedure TObject.FreeAndNil<T:class>(var Object:T); inline; // public generic class method
begin
  InternalFreeAndNil(TObject(Object));
end;

and to have the current (10.4 and earlier) FreeAndNil removed from the sysutils unit to avoid ambiguity.

When the new generic FreeAndNil method is called from within any other method, one can simply call:

FreeAndNil(SomeObjectVariable)

and 10.3+ type inference avoids having to write:

FreeAndNil<TMyClassSpec>(SomeObjectVariable)

which is nice because most of your code will compile nicely without a change.

In some other spots, eg global routines and initialization / finalization sections one would have to call:

TObject.FreeAndNil(SomeObjectVariable)

Which to me would be acceptable, and a lot better than the current and historical half-way solutions with a FreeAndNil(const [ref] aObject:TObject) or an untyped FreeAndNil(var aObject)

And since the routine is so utterly simple and performance appears to be an issue, one could argue to have an assembler implementation for it. Though I am not sure if this is allowed/possible for generic, (and preferably inline) methods.

FTM: One could also just keep FreeAndNil(var aObject:TObject) and tell people to do a typecast like below, which also avoids the compiler complaining about the var type. But in this case, probably a lot of source code has to be adjusted. On the other hand it saves on code bloat, still avoids Invalid use of function results, properties or invalid types like records and pointers as parameter to FreeAndNil, and is utterly simple to change/implement.

...
var Obj:=TSomeObject.Create;
try
   DoSOmethingUseFulWithObj(Obj);
finally
  FreeAndNil(TObject(Obj)); // typecast avoids compiler complaining. Compiler wont allow invalid typecasts
end;
...
Intangible answered 21/6, 2020 at 21:33 Comment(10)
While you can do whatever you suits you the best in your code, I don't like the ideas of using class helper nor adding FreeAndNil to TObject class in RTL. Class helper is not ideal because you can have only one in scope, so I don't like to use them on such global level because it may collide with another one. Official RTL implementation on TObject class would break backward compatibility without gaining too much.Rothman
Also having FreeAndNil on TObject allows mistakes like Foo.FreeAndNil(Bar) This is why I prefer having separate class if the option of having generic standalone procedure is out of reach (again, that would be the best solution for the future)Rothman
FreeAndNil generated code is simple enough and adding assembler version would not improve it. There are no performance issues in FreeAndNil. Problem with performance would arise only for your proposal of making FreeAndNil thread safe - which again is wrong solution to thread safety issues - FreeAndNil alone cannot make code thread safe.Rothman
@DalijaPrasnikar Can you elaborate on what would go wrong with Foo.FreeAndNil(Bar) or the potential mistake? It would simple destroy Bar if any, and set Bar to nil. FTM it would be quivalent to TFoo.FreeAndNil(Bar) since it is a class proc. It relies on type inference and generics to get the correct TObject.FreeAndNil<TInferredType>(var aObject:TInferredType). I Agree a global FreeAndNil<TInferredType> would be the nicest, but would require a compiler change. RTL Source change seems more likely to happen than compiler change, but who knows.Intangible
It could go wrong if you actually wanted to free Foo instead of Bar. If you read that code fast enough you can easily ignore actual parameter Bar because Foo has more weight while reading.Rothman
Class name TObj in my example is completely arbitrary... you can choose any name you like.Rothman
@DalijaPrasnikar : I think Foo.FreeAndNil(Bar) is indeed confusing and should be written as TFoo.FreeAndNil(Bar) in the first place. TObj is arbitrary indeed, and I like TObject so 1) we can avoid the class helper. And 2) since it is a class method (and not a regular method), it allows for a drop-in-replacement of the existing FreeAndNil and would not have impact on the virtual table AFAIK. 3) I don't expect the TObject interface change to break any existing object related code. But then again this might be a lack of my delphi experience.Intangible
Hope no-one is annoyed by my english variations :sIntangible
There is difference between "should" and "reality". That was the problem with first RTL FreeAndNil implementation, that is the problem with second one. I strongly oppose adding another half baked solution to RTL in terms of TObject.FreeAndNil<T> and next RTL should be proper one - standalone generic.Rothman
@DalijaPrasnikar Indeed. And that's why I voted your solution as primary solution.Intangible
R
7

The only proper solution to FreeAndNil that is both type safe and does not allow freeing function results and properties would be generic var parameter:

 procedure FreeAndNil<T: class>(var Obj: T); inline;

But, currently Delphi compiler does not allow generics on standalone procedures and functions https://quality.embarcadero.com/browse/RSP-13724

Still, that does not mean you cannot have generic FreeAndNil implementation, only that it will be a bit more verbose than necessary.

type
  TObj = class
  public
    class procedure FreeAndNil<T: class>(var Obj: T); static; inline;
  end;

class procedure TObj.FreeAndNil<T>(var Obj: T);
var
  Temp: TObject;
begin
  Temp := Obj;
  Obj := nil;
  Temp.Free;
end;

Type inference introduced in Rio will allow you to call it without specifying generic signature:

TObj.FreeAndNil(Obj);

Calling (and using) generic FreeAndNil in older Delphi versions is also possible but even more verbose

TObj.FreeAndNil<TFoo>(Obj);
Rothman answered 20/6, 2020 at 9:30 Comment(3)
Ah it was the type inference that allowed me to leave out the <T> for the SGFreeAndNil<T> of my TObjectHelper implementation, This allows using my SGFreeAndNil(FSomeFIeld) from within any object. Which covers about 99% of my code where (owned) objects are destroyed. Only in global routines (eg initialization/.finalization) would the speclal call of FreeAndNil have to be called as TObject.FreeAndNil(MyGLobalObject)Intangible
For the lack of generic var on global routines, I would make to suggestion to have an implementation of class procedure TObject.FreeAndNil<T>(var aObject:T) at the TObject class level and to deprecate the global FreeAndNil alltogether. This also avoids the need of a helper class at TObject level. And only when called from global routines one would need TObject.FreeAndNil(SomeObjectVariable). From within all object methods it would simply be FreeAndNil(SomeObject).Intangible
What I like less about this solution is that introduces a new class that you do not really need/use. Hence my solution below. For the current 10.4 one might implement this as a TObjectHelper =class helper for TObject class, for future versions it should be added to the TObject class.Intangible
I
2

Because we cannot create a global procedure FreeAndNil<T:class>(var aObject:T) I would suggest the code below as a method to the TObject class. (rtl change to be made by embarcadero, but does not need a compiler change)

class procedure TObject.InternalFreeAndNil(var Object:TObject); static; // strict private class method
begin
  if Assigned(Object) then
  begin
    var tmp:=Object;
    Object:=nil;
    tmp.Destroy;
  end;
end;


class procedure TObject.FreeAndNil<T:class>(var Object:T); inline; // public generic class method
begin
  InternalFreeAndNil(TObject(Object));
end;

and to have the current (10.4 and earlier) FreeAndNil removed from the sysutils unit to avoid ambiguity.

When the new generic FreeAndNil method is called from within any other method, one can simply call:

FreeAndNil(SomeObjectVariable)

and 10.3+ type inference avoids having to write:

FreeAndNil<TMyClassSpec>(SomeObjectVariable)

which is nice because most of your code will compile nicely without a change.

In some other spots, eg global routines and initialization / finalization sections one would have to call:

TObject.FreeAndNil(SomeObjectVariable)

Which to me would be acceptable, and a lot better than the current and historical half-way solutions with a FreeAndNil(const [ref] aObject:TObject) or an untyped FreeAndNil(var aObject)

And since the routine is so utterly simple and performance appears to be an issue, one could argue to have an assembler implementation for it. Though I am not sure if this is allowed/possible for generic, (and preferably inline) methods.

FTM: One could also just keep FreeAndNil(var aObject:TObject) and tell people to do a typecast like below, which also avoids the compiler complaining about the var type. But in this case, probably a lot of source code has to be adjusted. On the other hand it saves on code bloat, still avoids Invalid use of function results, properties or invalid types like records and pointers as parameter to FreeAndNil, and is utterly simple to change/implement.

...
var Obj:=TSomeObject.Create;
try
   DoSOmethingUseFulWithObj(Obj);
finally
  FreeAndNil(TObject(Obj)); // typecast avoids compiler complaining. Compiler wont allow invalid typecasts
end;
...
Intangible answered 21/6, 2020 at 21:33 Comment(10)
While you can do whatever you suits you the best in your code, I don't like the ideas of using class helper nor adding FreeAndNil to TObject class in RTL. Class helper is not ideal because you can have only one in scope, so I don't like to use them on such global level because it may collide with another one. Official RTL implementation on TObject class would break backward compatibility without gaining too much.Rothman
Also having FreeAndNil on TObject allows mistakes like Foo.FreeAndNil(Bar) This is why I prefer having separate class if the option of having generic standalone procedure is out of reach (again, that would be the best solution for the future)Rothman
FreeAndNil generated code is simple enough and adding assembler version would not improve it. There are no performance issues in FreeAndNil. Problem with performance would arise only for your proposal of making FreeAndNil thread safe - which again is wrong solution to thread safety issues - FreeAndNil alone cannot make code thread safe.Rothman
@DalijaPrasnikar Can you elaborate on what would go wrong with Foo.FreeAndNil(Bar) or the potential mistake? It would simple destroy Bar if any, and set Bar to nil. FTM it would be quivalent to TFoo.FreeAndNil(Bar) since it is a class proc. It relies on type inference and generics to get the correct TObject.FreeAndNil<TInferredType>(var aObject:TInferredType). I Agree a global FreeAndNil<TInferredType> would be the nicest, but would require a compiler change. RTL Source change seems more likely to happen than compiler change, but who knows.Intangible
It could go wrong if you actually wanted to free Foo instead of Bar. If you read that code fast enough you can easily ignore actual parameter Bar because Foo has more weight while reading.Rothman
Class name TObj in my example is completely arbitrary... you can choose any name you like.Rothman
@DalijaPrasnikar : I think Foo.FreeAndNil(Bar) is indeed confusing and should be written as TFoo.FreeAndNil(Bar) in the first place. TObj is arbitrary indeed, and I like TObject so 1) we can avoid the class helper. And 2) since it is a class method (and not a regular method), it allows for a drop-in-replacement of the existing FreeAndNil and would not have impact on the virtual table AFAIK. 3) I don't expect the TObject interface change to break any existing object related code. But then again this might be a lack of my delphi experience.Intangible
Hope no-one is annoyed by my english variations :sIntangible
There is difference between "should" and "reality". That was the problem with first RTL FreeAndNil implementation, that is the problem with second one. I strongly oppose adding another half baked solution to RTL in terms of TObject.FreeAndNil<T> and next RTL should be proper one - standalone generic.Rothman
@DalijaPrasnikar Indeed. And that's why I voted your solution as primary solution.Intangible

© 2022 - 2024 — McMap. All rights reserved.