Recent

Author Topic: PSA: Don't use FreeAndNil  (Read 16818 times)

af0815

  • Hero Member
  • *****
  • Posts: 1382
Re: PSA: Don't use FreeAndNil
« Reply #15 on: May 15, 2023, 04:24:58 pm »
This sets the pointer to be all bits being 1, this is (at least on all modern x64 systems) an invalid address, so you would get a crash. But thats just a hack. Nil is specifically built for being a pointer with no valid memory behind it, so we basically just re-invent nil because we can't use nil
Per Design, the Value of nil must not 0 (Zero 0). Every other value which is not a valid pointer should useable. A better value is deadbeef or some of this nice acronyms. But it must not depend on the value (Zero 0). The name itself says enough nil = not in list.

But this are my (old) 2 cents. (BTW, i have enough popcorn beside me lol )
regards
Andreas

Fred vS

  • Hero Member
  • *****
  • Posts: 3482
    • StrumPract is the musicians best friend
Re: PSA: Don't use FreeAndNil
« Reply #16 on: May 15, 2023, 04:28:26 pm »
An other option may be somthing like this:
Code: Pascal  [Select][+][-]
  1. procedure FreeAndInvald(var p: TObject);
  2. begin
  3.   p.Free;
  4.   p := TObject(not 0);
  5. end;

Hello Warfley.

With object like this, i can use FreeAndNil():
Code: Pascal  [Select][+][-]
  1. type
  2.   Tmyobject = class(TObject)
  3.     public
  4.       ...
  5.    end;
  6.  
  7. var
  8.   Myobject : Tmyobject;
  9.  
  10. ...
  11.   FreeAndNil(Myobject); // no cry

But using your code:
Code: Pascal  [Select][+][-]
  1.     procedure FreeAndInvald(var p: TObject);
  2.     begin
  3.       p.Free;
  4.       p := TObject(not 0);
  5.     end;

Replacing FreeAndNil with FreeAndInvald
   
Code: Pascal  [Select][+][-]
  1. FreeAndInvald(Myobject);

there is that error at compilation:
Quote
Error: (3069) Call by var for arg no. 1 has to match exactly: Got "Tmyobject" expected "TObject"

 :-\


 
I use Lazarus 2.2.0 32/64 and FPC 3.2.2 32/64 on Debian 11 64 bit, Windows 10, Windows 7 32/64, Windows XP 32,  FreeBSD 64.
Widgetset: fpGUI, MSEgui, Win32, GTK2, Qt.

https://github.com/fredvs
https://gitlab.com/fredvs
https://codeberg.org/fredvs

Warfley

  • Hero Member
  • *****
  • Posts: 1865
Re: PSA: Don't use FreeAndNil
« Reply #17 on: May 15, 2023, 06:40:53 pm »
Per Design, the Value of nil must not 0 (Zero 0). Every other value which is not a valid pointer should useable. A better value is deadbeef or some of this nice acronyms. But it must not depend on the value (Zero 0). The name itself says enough nil = not in list.

But this are my (old) 2 cents. (BTW, i have enough popcorn beside me lol )
This is the problem with re-inventing nil, nil is already exactly what we want, so making the same again is going to be a dirty hack at all times. The only reason this example I posted should work (must admit haven't tested it as I'm currently not on a PC with FPC installed), is because modern 64 bit MMUs actually only support for 48 bit address ranges, so there never can be a valid memory address at this point.

Code: Pascal  [Select][+][-]
  1. FreeAndInvald(Myobject);

there is that error at compilation:
Quote
Error: (3069) Call by var for arg no. 1 has to match exactly: Got "Tmyobject" expected "TObject"

 :-\
Ah yes I forgot about this. Inheritance downcasting does not work with var parameters. Thats the reason why the original FreeAndNil uses an untyped var. So either generics must be used, or it must be implemented like the original FreeAndNil:
Code: Pascal  [Select][+][-]
  1.  procedure FreeAndNil(var obj);
  2.       var
  3.         temp: tobject;
  4.       begin
  5.         temp:=tobject(obj);
  6.         pointer(obj):=nil;// Just like this but instead of nil here an invalid pointer value
  7.         temp.free;
  8.       end;

Fred vS

  • Hero Member
  • *****
  • Posts: 3482
    • StrumPract is the musicians best friend
Re: PSA: Don't use FreeAndNil
« Reply #18 on: May 15, 2023, 06:57:35 pm »
Quote
Error: (3069) Call by var for arg no. 1 has to match exactly: Got "Tmyobject" expected "TObject"
Ah yes I forgot about this. Inheritance downcasting does not work with var parameters. Thats the reason why the original FreeAndNil uses an untyped var. So either generics must be used, or it must be implemented like the original FreeAndNil:
[/quote]

Ha, ok, must be done in sysutils.inc and recompile fpc because obj is declared there.

Thanks for all that precious info.

Fre;D
I use Lazarus 2.2.0 32/64 and FPC 3.2.2 32/64 on Debian 11 64 bit, Windows 10, Windows 7 32/64, Windows XP 32,  FreeBSD 64.
Widgetset: fpGUI, MSEgui, Win32, GTK2, Qt.

https://github.com/fredvs
https://gitlab.com/fredvs
https://codeberg.org/fredvs

Warfley

  • Hero Member
  • *****
  • Posts: 1865
Re: PSA: Don't use FreeAndNil
« Reply #19 on: May 15, 2023, 07:03:33 pm »
Don't change FreeAndNil, or you may break some parts of the code that rely on that proerpty of Free and Nil. Just create your own alternative, but you can copy the code from FreeAndNil and just change it up

ASerge

  • Hero Member
  • *****
  • Posts: 2383
Re: PSA: Don't use FreeAndNil
« Reply #20 on: May 16, 2023, 04:55:24 pm »
FreeAndNil is useful when there are cross-references: parent/child, container/content.
Also, zeroing a reference to an object quickly detects an error accessing the fields of an already destroyed object, since accessing the null address is an error in almost all operating systems. But without it, it can go unnoticed for a long time.
I always use Free for local variables, and for global ones, especially those visible from the outside - most often FreeAndNil.

Warfley

  • Hero Member
  • *****
  • Posts: 1865
Re: PSA: Don't use FreeAndNil
« Reply #21 on: May 16, 2023, 06:22:53 pm »
Also, zeroing a reference to an object quickly detects an error accessing the fields of an already destroyed object, since accessing the null address is an error in almost all operating systems. But without it, it can go unnoticed for a long time.
But thats the point, with the other tooling at hand, like Valgrind or Heaptrc (with keepreleased true), you can detect these errors also very easiely, with the added bonus of being able to detect double Free Bugs, which FreeAndNil will completely hide. If you are using Valgrind and Heaptrc, you already get all the features from FreeAndNil, and more, and using FreeAndNil in addition actually makes those tools less useful because it hides double free bugs, which these tools would otherwise detect.

There is an argument to be made in Tests vs Production code, because both HeapTrc and Valgrind are not feasable to do in production code (Valgrind emulates your code giving it a slowdown of a factor 1000 or so), so you may want to use it in production code, but not in debug or test builds.

But even than, a DestroyAndNil:
Code: Pascal  [Select][+][-]
  1. procedure DestroyAndNil(var obj);
  2. var
  3.   temp: tobject;
  4. begin
  5.   temp:=tobject(obj);
  6.   pointer(obj):=nil;
  7.   temp.destroy;
  8. end;
would be preferential here, because it detects use after free and double free (unlike FreeAndNil, which actively hides double free).

Remy Lebeau

  • Hero Member
  • *****
  • Posts: 1471
    • Lebeau Software
Re: PSA: Don't use FreeAndNil
« Reply #22 on: May 16, 2023, 07:13:38 pm »
There are only 1.01 problems with FreeAndNil:

1) Its argument is untyped. Changing your object type to a non-class will crash in a disgusting way, at best. If your new type is a static record that has an unrelated class instance at offset 0, things will be even worse...

Inheritance downcasting does not work with var parameters. Thats the reason why the original FreeAndNil uses an untyped var. So either generics must be used, or it must be implemented like the original FreeAndNil

Delphi (finally) addressed this very issue in 10.4 Sydney, by changing the signature of FreeAndNil() to this:

Code: Pascal  [Select][+][-]
  1. procedure FreeAndNil(const [ref] Obj: TObject);

It is no longer possible to pass in anything that is not a class object pointer, and it does allow passing in (a reference to) a pointer to any type that is derived from TObject.

See: Magic behind FreeAndNil for an explanation.

Arguably using Generics would have been a better option, but they went with this solution instead, probably for performance reasons.

Perhaps FreePascal could implement an equivalent solution?
« Last Edit: May 16, 2023, 07:20:36 pm by Remy Lebeau »
Remy Lebeau
Lebeau Software - Owner, Developer
Internet Direct (Indy) - Admin, Developer (Support forum)

PascalDragon

  • Hero Member
  • *****
  • Posts: 5870
  • Compiler Developer
Re: PSA: Don't use FreeAndNil
« Reply #23 on: May 18, 2023, 05:22:42 pm »
Per Design, the Value of nil must not 0 (Zero 0). Every other value which is not a valid pointer should useable. A better value is deadbeef or some of this nice acronyms. But it must not depend on the value (Zero 0). The name itself says enough nil = not in list.

But this are my (old) 2 cents. (BTW, i have enough popcorn beside me lol )
This is the problem with re-inventing nil, nil is already exactly what we want, so making the same again is going to be a dirty hack at all times. The only reason this example I posted should work (must admit haven't tested it as I'm currently not on a PC with FPC installed), is because modern 64 bit MMUs actually only support for 48 bit address ranges, so there never can be a valid memory address at this point.

While x64 CPU MMUs support only a specific range of bits the thing is that the available 48-bit address range is split into an upper and lower range: addresses with all leading bits 0 and addresses with all leading bits 1 with a gap of invalid addresses located between those two areas. Most OS will put the user space memory in the lower area and the kernel space memory in the upper one, but that is definitely not a requirement (e.g. in the OS we develop at my company the kernel resides in the lower area and the user space in the upper area). That means depending on the OS the address $ffffffff_ffffffff can be valid for a user space application.

Arguably using Generics would have been a better option, but they went with this solution instead, probably for performance reasons.

Unlike FPC Delphi does not support generics for global functions so they'd have to implement that first and I think they're not interested in that.

Perhaps FreePascal could implement an equivalent solution?

The idea to use constref is not possible, because the optimizer (especially if based on LLVM) might optimize this code with the knowledge that the parameter can not be changed.

What would be possible would be to declare a generic overload of FreeAndNil in the SysUtils unit that would be automatically picked up if the user enables ImplicitFunctionSpecializations as the compiler will prefer the generic instead of the function with the less precise formal parameter of the non-generic FreeAndNil (of course one can still specialize it manually).

A future optimization would then need to ensure that the various FreeAndNil specializations are then folded into only one in the final binary...

BeniBela

  • Hero Member
  • *****
  • Posts: 921
    • homepage
Re: PSA: Don't use FreeAndNil
« Reply #24 on: May 19, 2023, 01:37:23 am »
Per Design, the Value of nil must not 0 (Zero 0). Every other value which is not a valid pointer should useable. A better value is deadbeef or some of this nice acronyms. But it must not depend on the value (Zero 0). The name itself says enough nil = not in list.

But this are my (old) 2 cents. (BTW, i have enough popcorn beside me lol )
This is the problem with re-inventing nil, nil is already exactly what we want, so making the same again is going to be a dirty hack at all times. The only reason this example I posted should work (must admit haven't tested it as I'm currently not on a PC with FPC installed), is because modern 64 bit MMUs actually only support for 48 bit address ranges, so there never can be a valid memory address at this point.

While x64 CPU MMUs support only a specific range of bits the thing is that the available 48-bit address range is split into an upper and lower range: addresses with all leading bits 0 and addresses with all leading bits 1 with a gap of invalid addresses located between those two areas. Most OS will put the user space memory in the lower area and the kernel space memory in the upper one, but that is definitely not a requirement (e.g. in the OS we develop at my company the kernel resides in the lower area and the user space in the upper area). That means depending on the OS the address $ffffffff_ffffffff can be valid for a user space application.

I wonder what aarch64 does. I just spent the last weekend building a tagged pointer variant type.

The 16 unused bits of the 64-bit pointer  are perfect to store some tagging data.


That means depending on the OS the address $ffffffff_ffffffff can be valid for a user space application.

but only for single bytes, so not for TObject

Warfley

  • Hero Member
  • *****
  • Posts: 1865
Re: PSA: Don't use FreeAndNil
« Reply #25 on: May 19, 2023, 03:04:42 am »
The 16 unused bits of the 64-bit pointer  are perfect to store some tagging data.
I actually had a project once, where I had basically 2 fields, an enum of the type of data, and then a pointer to the data. As this was a very long running computation, that also required a lot of memory (i.e. a lot of these structures), I decided to just combine them in one pointer. This means that a lot of the loading and copying operations of such structures were just a single operation, while also saving like 4 whole bytes (1 for the enum and 3 for the padding). It's a desperate optimization but can be useful.

What I also found this useful for was actually for lockless datastructures (e.g. lockless double linked list), as you can use thse bits to tag if an object is currently in use (for being freed), which allows you to check this state together with the actual data within a single atomic CompareAndExchange instruction. Similar on how Harris did it in his 2001 paper, but Harris used a 32 bit system and used the fact that due to pointer alignment there was effectively one free bit. When I reimplemented this structure for x86_64 I simply used the unused bits for a 64 bit address

kupferstecher

  • Hero Member
  • *****
  • Posts: 603
Re: PSA: Don't use FreeAndNil
« Reply #26 on: May 19, 2023, 01:34:24 pm »
And the important part is that you want crashes. If something happens that you do not expect a crash is always better than ignoring it and doing nothing
To this statement I only agree in the develpopement phase, not in the production code.
In programm usage a crash imho is the worst of all szenarios. You gave the example of the problem with the unaccessable file. If the programm would crash because that situation isn't handled, than the one hour work really is lost. If the programm keeps working even in a crocked way, chances are that you manage to save your data in an other way. Of course there are situations when the crash is far better, but I'd say in most cases its the other way round.

Still I mostly don't use FreeAndNil, because I consider the use-after-free as the more common bug and for me less easy to detect. Less easy to detect, because it depends much more on the program flow than a double Free*. Members may be accessed from all over the programm while there are mostly only very few places a freeing occurs. And heaptrace and valgrind as far as I understand doesn't help if that buggy program part isn't executed, e.g. if it's a corner case of the code that is only reached in very specific circumstances.

So from your precondition that the program should rather crash even on the customers machine, I think FreeAndNil would be the option with more "hits".

Perhaps it would be desireable to have a compiler option to change the behaviour of Free() between Free and FreeAndNil so that both versions could be used while testing the programm without rewriting the code. Because a "bug free" program (bug free in that regards) has to work in both ways of freeing.

*There are cases when short living objects are created and freed heavily in shared varibles, then I rather go with the FreeAndNil (explicitly in these cases), so that the state of the variable is more clear and can be used for additional checks, while the management of the object doesn't rely on that.

[Edit: Thanks for this topic and the details, it's really interesting and makes me think about my coding.]
« Last Edit: May 19, 2023, 01:38:25 pm by kupferstecher »

Swami With The Salami

  • Guest
Re: PSA: Don't use FreeAndNil
« Reply #27 on: May 19, 2023, 05:37:39 pm »
Why would anybody use FreeAndNil on a local variable? It doesn’t make sense.

If you have a large work-horse class represented by a variable that is available throughout the lifetime of your program and you want to instantiate it only when it is needed and destroy it as soon as you can your code would look like this:

Code: Pascal  [Select][+][-]
  1. if FWorkHose <> nil then
  2. begin
  3.   FWorkHose := TWorkHorse.Create;
  4.   try
  5.     ...
  6.   finally
  7.     FWorkHose.Free;
  8.     FWorkHorse := nil;
  9.   end;
  10. end;

FreeAndNil was introduced sometime around Delphi 3 or 4. It was a bonehead idea in my opinion. But I am not the sharpest knife. Can anybody give me another example of why I would want to set a variable to nil?

lainz

  • Hero Member
  • *****
  • Posts: 4689
  • Web, Desktop & Android developer
    • https://lainz.github.io/
Re: PSA: Don't use FreeAndNil
« Reply #28 on: May 19, 2023, 05:54:38 pm »
Why would anybody use FreeAndNil on a local variable? It doesn’t make sense.

If you have a large work-horse class represented by a variable that is available throughout the lifetime of your program and you want to instantiate it only when it is needed and destroy it as soon as you can your code would look like this:

Code: Pascal  [Select][+][-]
  1. if FWorkHose <> nil then
  2. begin
  3.   FWorkHose := TWorkHorse.Create;
  4.   try
  5.     ...
  6.   finally
  7.     FWorkHose.Free;
  8.     FWorkHorse := nil;
  9.   end;
  10. end;

FreeAndNil was introduced sometime around Delphi 3 or 4. It was a bonehead idea in my opinion. But I am not the sharpest knife. Can anybody give me another example of why I would want to set a variable to nil?

Sorry, I don't understand your code, it should be:

Code: Pascal  [Select][+][-]
  1. if FWorkHose = nil then

Or I'm wrong?

Swami With The Salami

  • Guest
Re: PSA: Don't use FreeAndNil
« Reply #29 on: May 19, 2023, 06:00:32 pm »
Yes, you're absolutely right - thanks for pointing that out. Sorry.

 

TinyPortal © 2005-2018