Recent

Author Topic: How to avoid nested try...finally...end?  (Read 6605 times)

bpranoto

  • Full Member
  • ***
  • Posts: 134
How to avoid nested try...finally...end?
« on: March 23, 2017, 03:04:09 pm »
try..finally...end is meant to ensure all created object be freed if an exception occurs.

Let's say we have 3 object to be created, the code will look like this:

Code: Pascal  [Select][+][-]
  1. procedure Oops1;
  2. var
  3.   x: TSomeObject;
  4.   y: TSomeObject;
  5.   z: TSomeObject;
  6. begin
  7.   x := TSomeObject.Create;
  8.   try
  9.     x.DoSomeThing
  10.     y := TSomeObject.Create;
  11.     try
  12.       y.DoSomeThing;
  13.       z := TSomeObject.Create;
  14.       try
  15.         z.DoSomeThing;
  16.       finally
  17.         z.Free;
  18.       end;
  19.     finally
  20.       y.Free;
  21.     end;
  22.   finally
  23.     x.Free;  
  24.   end;
  25. end;

Is there a better way than the nested try..finally..end?

marcov

  • Administrator
  • Hero Member
  • *
  • Posts: 11446
  • FPC developer.
Re: How to avoid nested try...finally...end?
« Reply #1 on: March 23, 2017, 03:21:41 pm »
In general, the best way is to simply make sure that no exceptions can happen, and handle the ones that do locally and don't let them climb up the hierarchy.

This might already be partially the case, if e.g. TSomeObject.create and .free can't really raise exceptions (except out of memory which is doubtful, and the program is unlikely to survive it anyway) it reduces to

Code: Pascal  [Select][+][-]
  1.       x := TSomeObject.Create;
  2.       y := TSomeObject.Create;
  3.       z := TSomeObject.Create;
  4.       try
  5.         x.DoSomeThing;
  6.         y.DoSomeThing;
  7.         z.DoSomeThing;
  8.       finally
  9.        x.free;
  10.        y.free;
  11.        z.free;
  12.     end;
  13.  

There are also some schemes in circulation that abuse interfaces for auto free schemes. I always found them unpractical.

Thaddy

  • Hero Member
  • *****
  • Posts: 14361
  • Sensorship about opinions does not belong here.
Re: How to avoid nested try...finally...end?
« Reply #2 on: March 23, 2017, 04:42:06 pm »
I have never seen any objections for the nested try/finally's especially when classes depend on each other.
OTOH Marco's example is a valid one, but lacks a little defensive programming if the class instances depend on each other. I would suggest to combine Marco's example with assertions (Assert(Assigned..) during development. Assertions disappear when turned off for a production build. My opinion is that if it is just for shorthand, think again. This will not help, though, if the creation depends on a dynamic code path.

Anyway. There's ARC in the pipeline and that will solve it partially.
You could also consider something like this (note: may be trunk only!):
Code: Pascal  [Select][+][-]
  1. unit smartptrs;
  2. { free interpretation - with very few changes! - from an example by Marco Cantu}
  3. {$ifdef fpc}{$mode delphi}{$endif}
  4. interface
  5.  
  6. type
  7.   Smart<T:class, constructor> = record
  8.   strict private
  9.     FValue:T;
  10.     FFreeTheValue:IInterface;
  11.     function GetValue:T;
  12.     type
  13.       TFreeTheValue = class(TInterfacedObject)
  14.       private
  15.         fObjectToFree:TObject;
  16.       public
  17.         constructor Create(anObjectToFree: TObject);
  18.         destructor Destroy;override;
  19.       end;
  20.    public
  21.      constructor Create(AValue: T);overload;
  22.      procedure Create;overload;
  23.      class operator Implicit(AValue: T):Smart<T>;
  24.      class operator Implicit(smart: Smart<T>):T;
  25.      property value: T read GetValue;
  26.    end;
  27. implementation
  28.  
  29.    constructor Smart<T>.TFreeTheValue.Create(anObjectToFree:TObject);
  30.    begin
  31.      self.fObjectToFree := anObjectToFree;
  32.    end;
  33.    
  34.    destructor Smart<T>.TFreeTheValue.Destroy;
  35.    begin
  36.      fObjectToFree.Free;
  37.      inherited;
  38.    end;
  39.    
  40.    constructor Smart<T>.Create(AValue:T);
  41.    begin
  42.      FValue := AValue;
  43.      FFreeTheValue := TFreeTheValue.Create(FValue);
  44.    end;
  45.    
  46.    procedure Smart<T>.Create;
  47.    begin
  48.      Smart<T>.Create(T.Create);
  49.    end;
  50.    
  51.    class operator Smart<t>.Implicit(AValue:T):Smart<T>;
  52.    begin
  53.      Result := Smart<T>.Create(AValue);
  54.    end;
  55.    
  56.    class operator Smart<T>.Implicit(smart: Smart<T>):T;
  57.    begin
  58.      Result := Smart.Value;
  59.    end;
  60.  
  61.    function Smart<T>.GetValue:T;
  62.    begin
  63.      if not Assigned(FFreeTheValue) then
  64.        Self := Smart<T>.Create(T.Create);
  65.      Result := FValue;
  66.    end;
  67. end.
Test:
Code: Pascal  [Select][+][-]
  1. program project1;
  2. {$mode delphi}{$apptype console}{$H+}
  3. uses classes, smartptrs;
  4.  
  5. var
  6.   a:TStrings;
  7.   b:Smart<TStringlist>;
  8.   c:Smart<TStringlist>;
  9. begin
  10.   a := b.value;
  11.   a.add('test');
  12.   writeln(a.text);
  13.   c.value.Add('test some more');
  14.   writeln(c.value.text);
  15.   readln;
  16.  // all cleanup is automatic
  17. end.  


Not really impractical then? Anyway. We will get something even "better" in the future. (I still have doubts about ARC)
« Last Edit: March 23, 2017, 04:54:35 pm by Thaddy »
Object Pascal programmers should get rid of their "component fetish" especially with the non-visuals.

marcov

  • Administrator
  • Hero Member
  • *
  • Posts: 11446
  • FPC developer.
Re: How to avoid nested try...finally...end?
« Reply #3 on: March 23, 2017, 04:46:36 pm »
Anyway. There's ARC in the pipeline and that will solve it partially.

No it will just move the problem to placing [weak]'s correctly for non-trivial code, and debugging the cases that you forgot.
« Last Edit: March 23, 2017, 04:54:38 pm by marcov »

bpranoto

  • Full Member
  • ***
  • Posts: 134
Re: How to avoid nested try...finally...end?
« Reply #4 on: March 23, 2017, 04:48:19 pm »
Finally, I do this way:

Code: Pascal  [Select][+][-]
  1. x := NIL;
  2. y := NIL;
  3. z := NIL;
  4.  
  5. try
  6.  x := TSomeObject.Create;
  7.  x.DoSomeThing;
  8.  y := TSomeObject.Create;
  9.  y.DoSomeThing;
  10.  z := TSomeObject.Create;
  11.  z.DoSomeThing;
  12. finally
  13.  z.Free;
  14.  y.Free;
  15.  x.Free;
  16. end;
  17.  

Because all objects are initialized with NIL, freeing is not a problem eventhough the object is not created yet.

Fungus

  • Sr. Member
  • ****
  • Posts: 353
Re: How to avoid nested try...finally...end?
« Reply #5 on: March 23, 2017, 04:56:07 pm »
Finally, I do this way:

Code: Pascal  [Select][+][-]
  1. x := NIL;
  2. y := NIL;
  3. z := NIL;
  4.  
  5. try
  6.  x := TSomeObject.Create;
  7.  x.DoSomeThing;
  8.  y := TSomeObject.Create;
  9.  y.DoSomeThing;
  10.  z := TSomeObject.Create;
  11.  z.DoSomeThing;
  12. finally
  13.  z.Free;
  14.  y.Free;
  15.  x.Free;
  16. end;
  17.  

Because all objects are initialized with NIL, freeing is not a problem eventhough the object is not created yet.

Use FreeAndNil which will test for a non nil pointer:

Code: Pascal  [Select][+][-]
  1. try
  2. ...
  3. finally
  4.  FreeAndNil(z);
  5.  FreeAndNil(y);
  6.  FreeAndNil(x);
  7. end;

http://www.freepascal.org/docs-html/rtl/sysutils/freeandnil.html

bpranoto

  • Full Member
  • ***
  • Posts: 134
Re: How to avoid nested try...finally...end?
« Reply #6 on: March 23, 2017, 05:03:58 pm »
yes, FreeAndNil also do the tricks. Thanks.

Thaddy

  • Hero Member
  • *****
  • Posts: 14361
  • Sensorship about opinions does not belong here.
Re: How to avoid nested try...finally...end?
« Reply #7 on: March 23, 2017, 05:04:20 pm »
That's not a very good idea here, because FreeAndNil hides use-after-free errors.
I know you almost all in the Lazarus community like FreeAndNil, but it is bad programming: You won't be able to debug a use-after-free very easily, because you just made the pointer disappear..... Which is healing the symtom, not healing the cause. And what error is in the top 3 of common programming error by a landslide?
Indeed, use-after-free.

Every programmer introduces hard to find bugs at some point, but if you use FreeAndNil you did it on purpose... >:D
« Last Edit: March 23, 2017, 07:20:24 pm by Thaddy »
Object Pascal programmers should get rid of their "component fetish" especially with the non-visuals.

Kays

  • Hero Member
  • *****
  • Posts: 574
  • Whasup!?
    • KaiBurghardt.de
Re: How to avoid nested try...finally...end?
« Reply #8 on: March 23, 2017, 07:19:32 pm »
That's not a very good idea here, because FreeAndNil hides use-after-free errors. […] You won't be able to debug a use-after-free very easily, because you just made the pointer disappear […]
Huh? But apparently there's gotta be a second mean of access (“use”) to the object. So I still got something.

[…]I would suggest to combine Marco's example with assertions (Assert(Assigned..) during development.[…]
Mh. I'm doing assigned() checks after each create() and abort with proper error messages. It doesn't increase the nesting level for following code, and the trivial error of an unassigned pointer is handled at one spot (not distributed to the bottom of my code via finally…end). Also exception-handling is to be said to be quite “expensive”.

try..finally...end is meant to ensure all created object be freed if an exception occurs.
Just some brainstorming: Wrap more into functions/procedures (if it suits your case).
Yours Sincerely
Kai Burghardt

Thaddy

  • Hero Member
  • *****
  • Posts: 14361
  • Sensorship about opinions does not belong here.
Re: How to avoid nested try...finally...end?
« Reply #9 on: March 23, 2017, 07:28:33 pm »
That's not a very good idea here, because FreeAndNil hides use-after-free errors. […] You won't be able to debug a use-after-free very easily, because you just made the pointer disappear […]
Huh? But apparently there's gotta be a second mean of access (“use”) to the object. So I still got something.
An intermediate storage point? Or a heap allocation? Or a parameter? Or a thread, or mishaps with unit finalization order?
All too common. If that double pointer storage (which is what this means most of the time) is set to nil you can't reach it anymore and you will have an incomplete call-stack. That is called "debugging" the wrong way. Hide the symptom, avoid that you can fix the cause. Sooner than later that cause will blow up in your face.
Code: Pascal  [Select][+][-]
  1. b:=a; //for any type that needs finalization. Gets worse with pointers and interfaces...
That's enough....
« Last Edit: March 23, 2017, 07:34:20 pm by Thaddy »
Object Pascal programmers should get rid of their "component fetish" especially with the non-visuals.

Fungus

  • Sr. Member
  • ****
  • Posts: 353
Re: How to avoid nested try...finally...end?
« Reply #10 on: March 23, 2017, 07:40:47 pm »
That's not a very good idea here, because FreeAndNil hides use-after-free errors.
I know you almost all in the Lazarus community like FreeAndNil, but it is bad programming: You won't be able to debug a use-after-free very easily, because you just made the pointer disappear..... Which is healing the symtom, not healing the cause. And what error is in the top 3 of common programming error by a landslide?
Indeed, use-after-free.

Every programmer introduces hard to find bugs at some point, but if you use FreeAndNil you did it on purpose... >:D

Even though this sound like BS, there's grain of truth to this. FreeAndNil is declared as:

Code: Pascal  [Select][+][-]
  1. procedure FreeAndNil(var obj);
  2. var
  3.   temp: tobject;
  4. begin
  5.   temp:=tobject(obj);
  6.   pointer(obj):=nil;
  7.   temp.free;
  8. end;

And if an exception is raised in the destructor, the debugger will no longer know that it came from the argument (obj) and not the local "temp" variable. You could declare your own FreeAndNil to avoid this behaviour:

Code: Pascal  [Select][+][-]
  1. procedure FreeAndNilX(var obj); inline;
  2. begin
  3.   TObject(obj).Free;
  4.   Pointer(obj):= nil;
  5. end;

One can wonder why it is not done like that in sysutils... :-/

EDIT: The sysutils FreeAndNil should have been called NilAndFree for good measures! ;)
« Last Edit: March 23, 2017, 07:45:25 pm by Fungus »

Ondrej Pokorny

  • Full Member
  • ***
  • Posts: 220
Re: How to avoid nested try...finally...end?
« Reply #11 on: March 23, 2017, 08:44:59 pm »
1.) A constructor can raise exceptions. Why not, it's kind of function. Example: TFileStream.

2.) Free tests for a nil pointer as well.

3.) FreeAndNil is a completely valid concept and has its place for e.g. temporary objects.

4.) There is also FreeThenNil in lclproc.

rvk

  • Hero Member
  • *****
  • Posts: 6162
Re: How to avoid nested try...finally...end?
« Reply #12 on: March 23, 2017, 11:14:04 pm »
One can wonder why it is not done like that in sysutils... :-/
It's documented:
Quote
FreeAndNil will free the object in Obj and will set the reference in Obj to Nil. The reference is set to Nil first, so if an exception occurs in the destructor of the object, the reference will be Nil anyway.
http://www.freepascal.org/docs-html/rtl/sysutils/freeandnil.html

Additional reading material:
http://www.nickhodges.com/post/Using-FreeAndNil.aspx

And arguments from the other camp :D
http://eurekalog.blogspot.nl/2009/04/why-should-you-always-use-freeandnil_28.html
« Last Edit: March 23, 2017, 11:21:55 pm by rvk »

Remy Lebeau

  • Hero Member
  • *****
  • Posts: 1314
    • Lebeau Software
Re: How to avoid nested try...finally...end?
« Reply #13 on: March 24, 2017, 02:02:09 am »
Use FreeAndNil which will test for a non nil pointer:

TObject.Free() already checks for a nil pointer before calling the destructor.  So it is perfectly safe to call Free() on a nil object pointer.  There is no point in using FreeAndNil() unless you are going to reuse the same variables again later on and they actually need be nil by that time.
Remy Lebeau
Lebeau Software - Owner, Developer
Internet Direct (Indy) - Admin, Developer (Support forum)

ASerge

  • Hero Member
  • *****
  • Posts: 2240
Re: How to avoid nested try...finally...end?
« Reply #14 on: March 25, 2017, 06:48:29 am »
Let's say we have 3 object to be created, the code will look like this:
...
Is there a better way than the nested try..finally..end?
The same method as used for nested if, while, etc.: take out a part of code into a separate procedure.

 

TinyPortal © 2005-2018