Recent

Author Topic: Unhalted exeption with FreeAndNil  (Read 770 times)

freemind001

  • New Member
  • *
  • Posts: 35
Unhalted exeption with FreeAndNil
« on: March 26, 2023, 08:31:12 pm »
Code: Pascal  [Select][+][-]
  1.     try
  2.       IniFile := TIniFile.Create(aFilename, []);
  3.       IniFile.WriteString('Service', 'Type', 'simple');
  4.       ...
  5.       FreeAndNil(IniFile);
  6.     except
  7.       on E: Exception do
  8.       begin
  9.         Log(etError, 'Error creating ini file. ' + E.Message, False);
  10.         if Assigned(IniFile) then
  11.           FreeAndNil(IniFile);
  12.       end;
  13.     end;
If user has no write permission to aFileName an exception section is executed.
At that moment IniFile is assigned, but a new exception raizes "Unhalted exeption  occurred at ... blah-blah-blah" when I do FreeAndNil
What's wrong?

jamie

  • Hero Member
  • *****
  • Posts: 6091
Re: Unhalted exeption with FreeAndNil
« Reply #1 on: March 26, 2023, 08:44:26 pm »
Did you first NIL the infiile object at the start of the code?

The exception will exit the constructor before it gets to set it externally. So, what you have left is some wild pointer which will pass the Assignment test.

  It may not fix all of your problems but it sure would help.

The only true wisdom is knowing you know nothing

freemind001

  • New Member
  • *
  • Posts: 35
Re: Unhalted exeption with FreeAndNil
« Reply #2 on: March 26, 2023, 08:49:08 pm »
Code: Pascal  [Select][+][-]
  1.   var
  2.     IniFile: TIniFile = nil;
this way there is no exception while FreeAndNil, thank you.
« Last Edit: March 26, 2023, 08:54:22 pm by freemind001 »

korba812

  • Sr. Member
  • ****
  • Posts: 392
Re: Unhalted exeption with FreeAndNil
« Reply #3 on: March 26, 2023, 09:45:04 pm »
You can also use a nested try ... except ... finally block:
Code: Pascal  [Select][+][-]
  1. var
  2.   IniFile : TIniFile = nil;
  3. ...
  4.       try
  5.         try
  6.           IniFile := TIniFile.Create(aFilename, []);
  7.           IniFile.WriteString('Service', 'Type', 'simple');
  8.           ...
  9.         except
  10.           on E: Exception do
  11.             Log(etError, 'Error creating ini file. ' + E.Message, False);
  12.         end;
  13.       finally
  14.         if Assigned(IniFile) then
  15.           FreeAndNil(IniFile);
  16.       end;
  17.  

ArminLinder

  • Sr. Member
  • ****
  • Posts: 314
  • Keep it simple.
Re: Unhalted exeption with FreeAndNil
« Reply #4 on: March 26, 2023, 10:05:08 pm »
A variant, IMHO better catching the designer's intentions when they designed the try ... except construct:

Code: Pascal  [Select][+][-]
  1. function TForm1.SaveToConfig(Section, Key, Value: string): boolean;
  2.  
  3. var
  4.   iniFile: TIniFile;
  5.  
  6. begin
  7.   result := false;
  8.   try
  9.     IniFile := TIniFile.Create('Settings.ini', []);
  10.     try
  11.       IniFile.WriteString(Section, Key, Value);
  12.       result := true;
  13.     finally
  14.       FreeAndNil(IniFile);
  15.     end;
  16.   except
  17.     on E: Exception do
  18.     begin
  19.       DisplayError('Error writing ini file. ' + E.Message);
  20.     end;
  21.   end;
  22. end;

Looks a bit strange first time, but over the years I found no better way to handle exception checking for file accesses in Pascal, or generally, for situations wher a constructor may throw an exception. If an exception occurs after creation, The finally block is first executed and the file gracefully closed, and then the code *still* enters the exception handling block.

Another approach would be to use OOP, if iniFile is a class property, it gets automatically initialized NIL when the parent class is created.

Armin.
« Last Edit: March 26, 2023, 10:27:47 pm by Nimral »
Lazarus 3.3.2 on Windows 7,10,11, Debian 10.8 "Buster", macOS Catalina, macOS BigSur, VMWare Workstation 15, Raspberry Pi

jamie

  • Hero Member
  • *****
  • Posts: 6091
Re: Unhalted exeption with FreeAndNil
« Reply #5 on: March 26, 2023, 10:29:43 pm »
Hmm, Too much code...

here is a concept to think about.
Code: Pascal  [Select][+][-]
  1. unit Unit1;
  2.  
  3. {$mode objfpc}{$H+}
  4. {$ModeSwitch TypeHelpers}
  5.  
  6. interface
  7.  
  8. uses
  9.   Classes, SysUtils, Forms, Controls, Graphics, Dialogs, StdCtrls,inifiles;
  10.  
  11. type
  12.  TIniHelper = Class Helper for TiniFile
  13.    class Function SafeCreate(aFileName:String):TiniFile;
  14.  end;
  15.  
  16.   { TForm1 }
  17.  
  18.   TForm1 = class(TForm)
  19.     Button1: TButton;
  20.     procedure Button1Click(Sender: TObject);
  21.   private
  22.  
  23.   public
  24.  
  25.   end;
  26.  
  27. var
  28.   Form1: TForm1;
  29.  
  30. implementation
  31.  
  32. {$R *.lfm}
  33.  
  34. { TForm1 }
  35.  
  36. procedure TForm1.Button1Click(Sender: TObject);
  37. Var
  38.   IO:TIniFile;
  39. begin
  40. Try
  41.   IO := TiniFile.SafeCreate('Test');
  42.   //...
  43.   Finally
  44.   IO.Free;
  45. end;
  46. end;
  47.  
  48. class Function TiniHelper.SafeCreate(AFileName:String):TiniFile;
  49. Begin
  50.   Result := Nil;
  51.   Result := IniFiles.TIniFile.Create(AFileName, []);
  52. end;
  53.  
  54. end.
  55.  
  56.  
The only true wisdom is knowing you know nothing

Warfley

  • Hero Member
  • *****
  • Posts: 1499
Re: Unhalted exeption with FreeAndNil
« Reply #6 on: March 27, 2023, 12:21:55 am »
There is a rule of thumb with constructors, don't put constructors inside the try-except or try-finally block that frees them. Secondly, you should always use try-finally for freeing this not only avoids you having the two FreeAndNil in your code, but also frees on all other jumps such as Exit, Break or Continue. Once you use Finally you don't need to think about where you need to free anymore, because Finally will always be called

Always have your code as follows:
Code: Pascal  [Select][+][-]
  1. MyInstance := MyClass.Create;
  2. try
  3.   ...
  4. finally
  5.   MyInstance.Free;
  6. end;
The Create should never be in the same try-block that performs the free, because when a constructor fails, the memory is freed, but the result variable is in an undefined state (is btw true for any function, if a function throws an exception never touch the result). So if the Create fails in this construct, the exception will be raised and the try-finally block will never be executed and therefore the Free will not be called.

Therefore looking at the whole picture, Nimrals answer is the correct one.

Second:
Code: Pascal  [Select][+][-]
  1.         if Assigned(IniFile) then
  2.           FreeAndNil(IniFile);
This is a completely useless construct. Free is actually not the destructor, it is just a helper function (this indentation is in the original rtl source code):
Code: Pascal  [Select][+][-]
  1.       procedure TObject.Free;
  2.  
  3.         begin
  4.            // the call via self avoids a warning
  5.            if self<>nil then
  6.              self.destroy;
  7.         end;  
If you call free you don't need to check assigned, if you check assigned you can skip calling free and call destroy instead.

Lastly, personally I do not like FreeAndNil, because while it is intended to make your code more safe, it often does the complete opposite. When you use FreeAndNil, it will set the instance to nil, so when you call Free a second time on it, the check of the helper seen above will kick in, and it just jumps over it. So nice, no error on double free. But thats a big problem, because if you call Free twice, this is usually indicative of some assumptions not matching up, resulting in the state that you think you need to free but the data is already freed. This means that you have an error somewhere before. So by not crashing, and just swallowing it anyway, you miss that error and might not find that bug, until it crashes really hard.

So what would actually be better would be something like this:
Code: Pascal  [Select][+][-]
  1. procedure FreeAndFill(var instance: TObject);
  2. begin
  3.   if not Assigned(Instance) then
  4.     raise EDoubleFreeError.Create;
  5.   instance.Destroy;
  6.   Instance := nil;
  7. end;
Or alternatively, to set the instance not to nil, but to anohter invalid pointer that will segfault, e.g. -1 (highest possible address).

Because not crashing on error is the worst kind of error condition you can have
« Last Edit: March 27, 2023, 12:33:48 am by Warfley »

jamie

  • Hero Member
  • *****
  • Posts: 6091
Re: Unhalted exeption with FreeAndNil
« Reply #7 on: March 27, 2023, 12:51:13 am »
and for the shortest code block! :D

Code: Pascal  [Select][+][-]
  1. procedure TForm1.Button1Click(Sender: TObject);
  2. begin
  3. Try
  4.   With TiniFile.Create('Test') do
  5.    begin
  6.      WriteString('SectionName','TheValueName','TheValue');
  7.      Free;
  8.    end;
  9. Except //if you need it.
  10. end;
  11. end;              
  12.  

That just about the shortest way I can come up with!

It covers it all ! :o
The only true wisdom is knowing you know nothing

wp

  • Hero Member
  • *****
  • Posts: 11858
Re: Unhalted exeption with FreeAndNil
« Reply #8 on: March 27, 2023, 12:57:35 am »
No, it leaves a memory leak when the WriteString fails in case of writeprotected ini file. try-finally is much more important than try-except. (And "eating exceptions" is really bad habit, although I do it myself sometimes)
« Last Edit: March 27, 2023, 01:04:20 am by wp »

Warfley

  • Hero Member
  • *****
  • Posts: 1499
Re: Unhalted exeption with FreeAndNil
« Reply #9 on: March 27, 2023, 01:23:31 am »
I actually like a with-try-finally construct, whee instead of begin end a try block is used. Saves the two lines for begin end
Code: Pascal  [Select][+][-]
  1. with TiniFile.Create('Test') do
  2. try
  3.   WriteString('SectionName','TheValueName','TheValue');
  4. finally
  5.   Free;
  6. end;
But that's one of my guilty pleasures, when I'm just to lazy to define a variable. But that shouldn't be an excuse

That said, scoped with for managed records and interfaces would be nice, this would even avoid the try finally free part, and make them quite useful aside from being hast syntactic sugar of questionable readability

jamie

  • Hero Member
  • *****
  • Posts: 6091
Re: Unhalted exeption with FreeAndNil
« Reply #10 on: March 27, 2023, 01:27:15 am »
I was just going to post that exact code, you beat me to it!  :D
The only true wisdom is knowing you know nothing

jamie

  • Hero Member
  • *****
  • Posts: 6091
Re: Unhalted exeption with FreeAndNil
« Reply #11 on: March 27, 2023, 01:28:42 am »
how about a double take!
Code: Pascal  [Select][+][-]
  1. procedure TForm1.Button1Click(Sender: TObject);
  2. begin
  3.  Try
  4.   With TiniFile.Create('Test') do
  5.    Try
  6.      WriteString('SectionName','TheValueName','TheValue');
  7.    finally
  8.     Free;
  9.   end;
  10. Except //if you need it.
  11. end;
  12. end;                              
  13.  
  14.  
The only true wisdom is knowing you know nothing

 

TinyPortal © 2005-2018