We can to use methods of an object to create others objects, or this
is a bad programming practice?
eg:
var
  obj1, obj2: TmyObject;
begin
  obj1 := tobj1.create; //ok
  obj2 := obj1.createObj; // is this recommended?
  try
  //...
  finally
    obj1.free;
    obj2.free;
  end;
end;

Why I ask this:
If not exists the variable obj2 in call obj1.createObj will happen a memory leak, right? Because there is not variable to release.

Marcos Douglas

On Mon, 5 Jul 2010 15:48:40 -0300
Marcos Douglas wrote:

Questions of style: just a personal opinion.

How else can one program using object orientation?

VAR
    t1, t2 : Text ;
BEGIN
    t1 := Text('abcdefghi') ;
    t2 := t1.section(3,7) ;
    t2.write ;       // 'cdefg', if I'm right ;-)
END

Or do you mean to copy a (referenced) object?

Why I ask this: If not exists the variable obj2 in call obj1.createObj will happen a memory leak, right? Because there is not variable to release.

Do you mean using a function as a statement? (for its so-called "side effect") *This*, in my opinion, is very bad programming practice. But it's just an opinion. First, a function that has effects is already bad, except for recording data on the object (eg metadata, or memoization). Second, if you really want it to *do* something in addition to *making* its return value, you should make this "anomaly" obvious by writing the effect in an external proc (called by the func). So that, when you need the effect but not the value, just call the proc.

Marcos Douglas

Denis
________________________________

vit esse estrany ☣

spir.wikidot.com

This is used very often like this, so yes this is fine.

Not exactly sure what you mean?

There are 2 problems that can happen:

1)
"obj1.createObj" returns no object and obj2 will be nil, then of course calling obj2.foo would crash.
If "obj1.createObj" can return nil, then you must check for this.

2)
"obj1.createObj" crashes, then you are still before the try, and obj1 will not be freed => so yes leak

You can either do 2 nested try finally, or the following:

var
   obj1, obj2: TmyObject;
begin
   // make sure they are both initialized
   // obj1.free works correct, if obj1 is nil
   obj1 := nil;
   obj2 := nil;
   try
     obj1 := tobj1.create;
     obj2 := obj1.createObj;
   //...
   finally
     obj1.free;
     obj2.free;
   end;
end;

What do you mean "a function has effects is already bad"? A function/procedure always has effects, don't?

Second, if you really want it to *do* something in addition to *making* its return value, you should make this "anomaly" obvious by writing the effect in an external proc (called by the func). So that, when you need the effect but not the value, just call the proc.

Well... I think something is smell bad with this code :) But I will try a better example:
Imagine a text file. Each line correspond an object. A parser creates these objects. So, in a loop:

for I := 0 to Lines.Count-1 do
begin
  line := Lines[I];
  //(...)
  obj := Parser.GetObj(line);
  try
    //(...)
  finally
     obj.Free;
  end;
end;

The object Parser creates obj through the line string. If the *line* is broke, Parser do not can create an object and an Exception is created; the *obj* never is nil because there are exception handling.

Do you have a better idea for this example?

Marcos Douglas

Hi Martin

None of these problems will happen, because I treat all them.

I use this technique a lot, but thanks :) Unfortunately, this is not the *problem*.

The ask is:
If a function creates, inside, a object and there is no variable to receive this pointer, will happen a memory leak?

In my eg before, this is Okay:
obj2 := obj.createObj;

The obj2 is a pointer to release.

But, like that:
obj1.createObj; //no variable to receive

...now, where is the pointer to release?

Marcos Douglas

That would be, if a crash happens inside obj1.createObj? Because otherwise the obj is stored in obj2.

If it happens in there, then handle it in there => there is result as variable.

function TMyObject.creatObj : TSomeObj;
begin
   result := TsomeObject.create;
end;
function TMyObject.creatObj : TSomeObj;
begin
   result := TsomeObject.create;
   try
     DoFooBar;
   except
     result.Free; // will not be used by caller, but be aware it has
still a value, which now is invalid
   end;
end;
In the line
   result := TsomeObject.create;

the only crash that can happen is inside "create" => a crash in the constructor is handled by the compiler, the object will be freed.

Martin

Thanks, but lets continue :)

In another words, get you method:

function TMyObject.creatObj : TSomeObj; begin result := TsomeObject.create; end;

If I have a variable myObj, I can do this:
someObj := myObj.createObj;
This is the same:
someObj := TsomeObject.create;

Rigth?

But if a remove the variable *someObj* and call like this:
myObj.createObj; // I _removed_ the *someObj* here!!!

What will happen with the "result" variable? Is it released by the compiler?

Marcos Douglas

No. This will create a mem leak.

Mattias

that is the same as if you wrote

   TsomeObject.create;

without assigning the created object to a variable. Both cause a memory leek.

Mind you just because "Create" is a constructor does not protect it from being called without using the result.

Martin

Mattias Gaertner wrote

No. This will create a mem leak.

Thanks Mattias, I expect that...

Yes, as Mattias said too.

This type of construction should be banned, no?
Different this, some times I use OUT parameters to force the caller to receive the result.

Marcos Douglas

No.

TForm1.Create(Application);

Application will free the TForm1.

Different this, some times I use OUT parameters to force the caller to receive the result.

Mattias

Marcos Douglas wrote

The ask is: If a function creates, inside, a object and there is no variable to receive this pointer, will happen a memory leak?

Yes you will have a memory leak. Easy way to test is to enable the heaptrc unit (compiler option -gh). On terminating the application, the heaptrc unit will dump memory usage output to the console window.

If the object returned is a reference counted object, then I think it will be freed (though not working with the interface instance itself is bad practice).

Remember FPC doesn't have garbage collection like Java or .NET. Rule of thumb is that if you created it, you need to free it.

I would even add to this that you need to guard for exceptions:

A:=TSomeClass.Create;
try
   // do stuff
finally
   A.Free; Make sure it is freed, even in case of exception.
end;

If your classes have more global scope, rule of thumb is to create objects in the form/datamodule's OnCreate event, and to free them in the OnDestroy event. (I always code the create and free statements together, so as not to forget this).

Michael.

If Memory allocation is the point of discussion and you need to create objects ...
You should nest the creations in order of usage.

In your declarations you needed bot Object1 and Object2 as TMyObject;

That said the proper usage would be:

obj1:=TMyObject.Create;
Try
  obj2:=TMyObject.Create;
  Try
    Obj1.DoSomething1;
    Obj2.DoSomething2;
   Finally
     FreeAndNil(Obj2);
   end;
 Finally
   FreeAndNil(Obj1);
end;

I would like to know what the purpose is for having an auxiliary constructor if they both reference createObj.

Anything other than nested Exception Handling is very poor practice.

What do you mean "a function has effects is already bad"? A function/procedure always has effects, don't?

IMHO he means that a function should only return a value and not change the state of an object, a variable or any other program is good practice but there are obvious cases where this rule makes no sense, for example boolean functions, that use the return value to indicate success/failure, etc.

R.

A question of terminology. "Effect" usually means the changes a "routine" (function or procedure) lets once it has completed its task. Indeed, any routine can do changes, but we can make a difference between changes in routine's own local scope (usually on the stack) that disappear, and changes on the outer worlds that remain. The latter are called effect and are the purpose of procedures. (Hope I'm clear ;-) A function's purpose is to construct and return a given value. For instance the purpose of parser.getLine(source, pos) is seemingly clear; if it does an effect in addition to returning lines, then this effect is called "side-effect" because this is often considered bad practice. There are good reasons for this, mine are:
* This is misleading: nothing in the apparent purpose of the func, and in its name, lets guess that there are permanent changes in the world's state.
* It makes it hard to reason about the program.
* It kills formal properties of functions that could be used to prove. For instance, the same func called twice with the same arguments may not return the same result.

I do not see the problem, since obj is explicitely free'ed...

Marcos Douglas

Denis
________________________________

vit esse estrany ☣

spir.wikidot.com

Wouldn't it be nice if we had a try..except..finally statement supported in FPC. All-in-one.

eg:

A := TSomeClass.Create;
try
  // do stuff
except
  // handle error
finally
  A.Free; //  it is freed, even in case of exception.
end;

This would save such a lot of typing and indentation.

In this particular case you don't need the you catch the exception, code will execute normally after the except block. You don't need the finally then.

Michael.

In our previous episode, Graeme Geldenhuys said:

Wouldn't it be nice...

Ah, the kiss-of-death phrase for a new feature request :-)

Graeme,
I didn't know that, thanks for the tip!

Michael,
I always do that, but thanks!

Hum... I do not agree. Why not this? See..

obj1:=TMyObject.Create;
obj2:=TMyObject.Create;
Try
   Obj1.DoSomething1;
   Obj2.DoSomething2;
Finally
  FreeAndNil(Obj1);
  FreeAndNil(Obj2);
end;

I would like to know what the purpose is for having an auxiliary constructor if they both reference createObj.

If your object have a simple constructor, ok... do not need... but did you see my example before? An object Parser, reading text in a file and creating objects through these lines of text?

Marcos Douglas

We talk about just *functions* or we talk about *methods* too? I agree with you, but in part... If a *function* should only return a value then, what do a *procedure*? What are the good pratice for procedures?

Marcos Douglas

In your example, if obj2:=TMyObject.Create fails (which will happen if resources are not available)... then the exception is raised and Obj1 never gets freed. So in an example where this method is re-entrant, you would see many instances of this occurring.

Nested exception handling is the only way to avoid memory from leaking.

Michael, what do you think about the following popular nested try-except-finally construct?

try
  try
    ...
  except
    ...
  end;
finally
  ...
end;

My understanding of your last statement is the above construct is superfluous. Did it what you mean? If yes, is it only true in objfpc mode?

Thanks.

The problem is somebody (not me) forget (a stupid example) put the variable to receive the instance of object.

Marcos Douglas

It depends what will do in "except/end" block...

Marcos Douglas

Better:

obj1 := nil;
obj2 := nil;
Try
  obj1 := TMyObject.Create;
  obj2 := TMyObject.Create;
  obj1.DoSomething1;
  obj2.DoSomething2;
finally
  obj1.Free;
  obj2.Free;
end;

The objectcs are protected. But is boring... :)
Everybody codify like that, afraid if resources are not available?

Marcos Douglas

An article from today:
http://www.monien.net/blog/index.php/2010/07/creating-multiple-objects-using-try-finally/

Luiz

Nope. If Obj2 failed to create you will have a problem with Obj2.Free. If you used FreeAndNil(Obj1) and FreeAndNil(Obj2) that might work around that one issue... But taking the real issue to the next level... What if something in Obj1.free raised an exception... Then you will not free Object2. The "best" way is with nested exception handling. Sorry :-)

In our previous episode, Andrew Brunner said:

Nope. That's why it is free and not destroy.

If you used FreeAndNil(Obj1) and FreeAndNil(Obj2) that might work around that one issue..

Doesn't matter either.

But taking the real issue to the next level... What if something in Obj1.free raised an exception... Then you will not free Object2. The "best" way is with nested exception handling. Sorry :-)

That's the problematic one I think.

Double Nope. You cannot access methods of a nil object. Nil.Free will in-it-and-of-it cause a read access violation.

Andrew Brunner wrote

Double Nope. You cannot access methods of a nil object.

You can, as long as such methods are not virtual (and "free" is not a virtual method).

Nil.Free will in-it-and-of-it cause a read access violation.

This is what free does:

           if selfnil then
             self.destroy;

So it will work fine when called on "nil".

Jonas

In our previous episode, Andrew Brunner said:

1. Did you test this? And then debug it?
2. Ask yourself why don't we call .destroy directly, and what the use of .free is over .destroy.

We talk about just *functions* or we talk about *methods* too?

It is meant for 'method functions', too. But I don't follow that rule strictly and try to use a descriptive name that indicates that a function effects something outside its scope.

I agree with you, but in part... If a *function* should only return a value then, what do a *procedure*? What are the good pratice for procedures?

Good naming and do what you like.

R.

Okay Marco, you explained to Andrew! ;-)

Can an exception happen in Free method?! :-O

Marcos Douglas

Like obj1.CreateObj() ? =)

> I agree with you, but in part... If a *function* should only return a > value then, what do a *procedure*? What are the good pratice for > procedures? Good naming and do what you like.

:) Okay... thanks... ;-)

Marcos Douglas

Marcos Douglas wrote

Can an exception happen in Free method?

It can happen in Destroy, which is called by Free.

Jonas

And do not have medicine for that, right?

Marcos Douglas

There's indeed not much you can do in such a case other than logging an error message.

Jonas

Ok!
Well... sincerely I only use this method when I not know if an object will be instanced or not (depends of algorithm).

Marcos Douglas

Like obj1.CreateObj() ? =)

Sounds a little generic for my liking, but at least you can guess that there is something more going on than just returning a value, so, yes. :)
I'd rather use something like obj2 := obj1.CreateSpecialTMyObjectInstance(); myself, which explains why I cannot use a default constructor to create obj2. But in the end it is all about personal style.

R.

Well... I think use a name as CreateSpecialTMyObjectInstance() is very especific... but, as you said, in the end it is all about personal style.

I think found a better way to my example. Instead to create an object using Parser.CreateMyObj(ATxt), e.g., maybe is better have an object (TMyObj) instantiated in Parser and the name of method would be Parser.GetMyObj(ATxt). The Parser fill the FMyObj field (atributte) using the ATxt parameter. Like as DataSet.FieldByName('column_name') returns a TField object.

I use this style a lot, but I wanted to hear some better ideas.

MD.

Wrong again Andrew. Calling .Free is safe (even if the instance variable is nil) because internally it checks if the instance exists before continuing with calling .Destroy on that instance. Delphi 101. ;-)

Marcos Douglas wrote

> That's the problematic one I think. Can an exception happen in Free method?!  :-O

Yes! I often have to debug those in our complex business objects.

Wrong again Andrew. Calling .Free is safe (even if the instance variable is nil) because internally it checks if the instance exists before continuing with calling .Destroy on that instance.  Delphi 101.  ;-)

It most certainly is not "safe". LOL. Free calls destroy.. Destroy may contain other frees and routines... You cannot guarantee that free will even return.

You got that right. Which is my point exactly. If free blows out w/o a nested exception handle to catch it... The entire object goes into limbo and will never be free and thus cause a memory leak. Tell me someone here realises this (other than me ;-) is the essential point of this entire thread or I'm going to be very disappointed...

Andrew Brunner wrote

It most certainly is not "safe".

I meant is as in it is safe to call MyObj.Free even if MyObj = nil. I answered based on the explicit question.

But yes, in your example where MyObj was an instance, the code in the destructor could still trip you up if it caused an exception of some kind.

Whew, Ok. Years back Delphi (the experience I'm drawing on) Object.Free could not be done. Now that I know FPC does the check for nil I probably won't be using FreeAndNil anymore. But this raises a large concern for me. That is the EXTRA if statement being called for every destruction of every object will lead to a decrease in high scale operations where Classes were chosen instead of data structures.

I think that who ever decided to add that if statement did so at the cost of performance for high-scale systems. That if statement would never be true if people coding knew what they were doing. I would certainly have a case for removing that if statement.

Andrew Brunner wrote

You got that right.  Which is my point exactly.  If free blows out w/o a nested exception handle to catch it... The entire object goes into limbo and will never be free and thus cause a memory leak.

Even with a try..except, if you call MyObj.Free and it causes an exception, you are screwed either way and it will always cause a memory leak. Best is no notify the user some how, or crash out of the program, or fire the programmer for not writing enough unit tests. :)

Andrew Brunner wrote

It most certainly is not "safe". LOL. Free calls destroy.. Destroy may contain other frees and routines... You cannot guarantee that free will even return.

the point was to call .Free on an object that might not have been initialized, not what Free ( or destroy, for the matter) would eventually call _after_, so it _is_ safe.

I guess (well, I just don't remember exactly) Free is safe because it is a class method, hence it _can_ be called on a non initialized object; and therefore can test the object instance against Nil and act consequently.

Someone correct me if I'm wrong on this... ;-)

Cheers, A.

Graeme Geldenhuys wrote

Even with a try..except, if you call MyObj.Free and it causes an exception, you are screwed either way and it will always cause a memory leak. Best is no notify the user some how, or crash out of the program, or fire the programmer for not writing enough unit tests. :)

No, if the nesting is done properly you could guarantee that Obj2.Free would be called even if Obj1.Free failed with some exception. And further, nested exception handling offers safeguarding against such memory leaks during destruction if done properly. To me this is the difference between good software and great software... How well it handles exceptions.

Delphi and Kylix has done the exact same thing for years. In this case FPC is just Delphi compatible.

As for the extra check causing a hit in performance - I can't say I have ever noticed it in our projects where we often create and destroy 80,000+ objects. It's a safety net, and a handy one.

Maybe you can put together a speed test app and post your results. One test calls .Free, the other test calls .Destroy

Well... I think use a name as CreateSpecialTMyObjectInstance() is very especific... but, as you said, in the end it is all about personal style.

Perhaps I have not made myself clear.
example (makes not much sense; just to explain my naming style): var Line1, Line2: TMyLine;
...
Line2 := Line1.CreateDottedRedLine();

R.

Okay, you're right. Nesting exception handling is the only way to avoid memory from leaking, in these cases.

And now? Your program failed (e.g.) using Free method... you assured there is no memory leaking (using nesting exceptions), but this is a totally _unexpected_ exception!
IMHO the program should be aborted, the user is notified, log this error... start again.

If the program is a web app, you can't abort immediately (no?)... but you'll do it sometime ;-)

I'm more concerned with programming errors that can not see immediately, such as forgetting to free some object. For me, an Exception is an exception! I did not expect and I need do something (if the program does not have its own treatment for what occurred).

Marcos Douglas

+1

In fact trying to handle exceptions, that aren't supposed *can* even make it worse.

There are 2 ways an exception can happen.

1) intended use to indicate some error => the programmer knows and can set "try" blocks exactly where needed.

2) exceptions that are unexpected. A malfunction in the program. => tell the user to save the data, if possible, and exit the app asap.

And in the 2nd case, mem leaks are no worry => the app is going to be closed, mem will be freed by the OS.
But trying to free mem, could just create even more havoc. Imagine the exception was because of some random memory access (some uninitialized pointer). The whole memory management could be corrupted. Trying to free memory could just cause more and more exceptions.

Martin

The caveat is that exceptions in the Destroy does not necessarily mean a memory leak. The exception object will be taken care of. So just make sure the Destroy methods free what they are supposed to free.

I must admit I am not a subscriber of any practice that allocates with out strategizing where the memory will be reclaimed. Everything I do is accounted for. So I just don't see any of these issues with memory. But because the power is in the developer... this issue is paramount to developing stable code.

If the program is a web app, you can't abort immediately (no?)... but you'll do it sometime  ;-)

Yes. My experience is predominately based on a Social Computer Server engine where it is just not advisable to leak memory. I can move it, allocate, deallocate but I just don't have any instances of where I see memory leaking. And if there are leaks rebooting the device or server software is just not an option.

other developers using FPC prospective - if they find themselves having memory issues a Managed development platform with garbage collection would be advisable.

Op 2010-07-07 00:14, Andrew Brunner het geskryf:

No, if the nesting is done properly you could guarantee that Obj2.Free would be called even if Obj1.Free failed with some exception.

No matter how many nested try..except or try..finally blocks you have, if you are trying to free the instance and it causes an exception, you might catch the exception, but you still can't free the object because it caused an exception the first time, and will probably keep causing an exception every time you try and free that instance. There is no way around that memory leak because you simply can't free the instance.

Here is an example:
---------------[ memory_leak.pas ]------------------------
// compile with: fpc -gh -gl memory_leak.pas
program memory_leak;
{$mode objfpc}{$H+}
uses
  Classes, SysUtils;
type
  TMyObject = class(TObject)
  private
    FList: TList;
    FMyName: string;
  public
    constructor Create;
    destructor Destroy; override;
    property MyName: string read FMyName write FMyName;
  end;

{ TMyObject }

constructor TMyObject.Create;
begin
  FList := TList.Create;
end;
destructor TMyObject.Destroy;
var
  i, j: integer;
begin
  i := 10;
  j := 0;
  j := i div j;
  FList.Free;
  inherited Destroy;
end;
var
  obj: TMyObject;
{$R *.res}
begin
  obj := TMyObject.Create;
  try
    try
      obj.MyName := 'Me';
      try
        writeln(obj.MyName);
      except
        on E: Exception do
          writeln('Oops'); // after this, the finally will still run
      end;
    finally
      obj.Free;
    end;
  except
    on E: Exception do
      obj.Free;   // this will just cause another exception
  end;
end.

------------------------------------------------

...and here is the output:

------------------------------------------------
$ ./memory_leak
Me
An unhandled exception occurred at $00000000004002C7 : EDivByZero : Division by zero
$00000000004002C7 line 32 of memory_leak.lpr
$000000000040FB2A

Heap dump by heaptrc unit
29 memory blocks allocated : 1071/1104
23 memory blocks freed : 819/848
6 unfreed memory blocks : 252
True heap size : 163840
True free heap : 162624
Should be : 162816
Call trace for block $00007F2DBBB6D1E0 size 128
$0000000000415CAA
$0000000000411214
$0000000000400CB0
$000000000040FB2A
$0000000000400178
Call trace for block $00007F2DBBB8DB40 size 40
$0000000000411214
$0000000000400CB0
$000000000040FB2A
$0000000000400178
Call trace for block $00007F2DBBB8DA80 size 20
$0000000000462F7E
$0000000000400CB0
$000000000040FB2A
$0000000000400178
Call trace for block $00007F2DBBB8D840 size 24
$000000000042DE9E
$0000000000400216 line 23 of memory_leak.lpr
$000000000040034F line 43 of memory_leak.lpr
$0000000000400178
Call trace for block $00007F2DBBB853E0 size 16
$0000000000400216 line 23 of memory_leak.lpr
$000000000040034F line 43 of memory_leak.lpr
$0000000000400178
Call trace for block $00007F2DBBB8D780 size 24
$000000000040034F line
------------------------------------------------

Regards,
- Graeme -

Op 2010-07-07 03:32, Martin het geskryf:

That is exactly what I was trying to explain to Andrew. If freeing an instance is what caused the exception, no amount of try..except or try..finally blocks will help you - you still can't free the instance, so better notify the user to try and save, then quit the app and start again.

Regards,
- Graeme -

Op 2010-07-07 05:25, Andrew Brunner het geskryf:

So just make sure the Destroy methods free what they are supposed to free.

What if that was the cause of the exception in the first place. :-) eg: Exception occurs in the destructor before all resource could be freed.

Regards,
- Graeme -

Ok. I think I understand the discrepancy in our reasoning. The issue is when exceptions are raised during Obj.Free. I think it would help if you would assume that all exceptions raised in Obj.Free were handled - since my assertion is all developers using a non-managed platform like FPC. I'm saying that because if memory is allocated, and deallocated you will not blow-out the memory manager in FPC. Meaning, if you catch your exceptions FATAL errors will not even occur and there will be NO NEED to worry about restarting an application. Its a clean way of thinking. I have no worries. When I call Obj.Free it can raise exceptions, and still recover to the calling methods - whether we are talking about ObjX.free or ObjX.DoSomething.

My basic point is that just because an exception is raised does not mean the method will blow-out. It will be handled. LOL... With exception handling (often nested) where required.

But, even using nested handled the memory may be impaired so, you need restarting the application.

Your application will continue running, if that is what you meant when you said not to worry. But the memory may not be 100%, even using nested handled.

MD.

The initial exception does not have to be in Free or somewhere in the code a statement as follows is executed, then all bets are off

UninitializedPointerToRandomAddress := SomeValue;

You may have changed any other structure to anything. There is no waty to predict if the app can even continue to run, it can cause any amount of subsequent crashes.
It could have destroyed the memory-managers structures of which memory is allocated. It could have modified other pointers, that now point to random addresses, it could have hit the stack and modified the return address, ....

With an exception like this, the rule is: Do as little as you can. Everything you do can make it worse. Call a function that relies on as little data as possible, and tries to simple save all user-data to an emergency file. Then exit

Meaning, if you catch your exceptions FATAL errors will not even occur and there will be NO NEED to worry about restarting an application.

Fatal errors don't alwasy need an exception in order to occur. They can occur even if all exceptions are caught.

They occur because with growing complexity of a software, the chances grow that the developper will introduce an error in the code. If you argue that is not going to happen to you, then my congratulations. You must get a rather huge salary (million?), because if you look at the list of exploits in professional software, you will see that it happens to every other professional developper.

No of course an exception doe snot mean, that all is gone.

But you always have to know what the code that threw the exception was supposed to do.
If I write my own code then some methods I wrote are not expected to raise an exception (including they do not call any code that could ). Then there is no need for try blocks around this code, because if it does throw an exception, then it is an exception that can not be recovered.

As I said in my initial mail:
- There are exceptions that are intentionally risen. They can and should be handled correctly.
- There are exception that stem from errors in the implementation, they are not excpeted, and there is no way to assert how much dameage they have done

This mail was only about the 2nd kind of exception

Martin

Martin wrote

On 07/07/2010 13:30, Andrew Brunner wrote: > Ok. I think I understand the discrepancy in our reasoning. The issue > is when exceptions are raised during Obj.Free. I think it would help

....

....

And I forgot to return to the initial point ....

While object.free can throw an exception, in most cases it is not expected to do so. That means with most objects (at least those, that I use, or write), if an exception occurs during destruction, then that clearly is an exception of the 2nd kind. Therefore an exception that occurs in free/destroy (in my apps) indicates a none recoverable situation => and hence no more worries about leaks => only worries about saving as much userdata as possible.

so here is the code again
   obj1 := nil;
   obj2 := nil;
   try
     obj1 := tobj1.create;
     obj2 := obj1.createObj;
   //...
   finally
     obj1.free;
     obj2.free;
   end;

if " obj1.free; " crashes, then yes obj2 is not going to be released. But as explained in this and my last mail: It should not be attempted to be released.

if " obj1.free; " crashed, then the state of the app is unknown, anything that the app does from now on could cause more havoc. trying to free obj2 could destroy value-able user-data that otherwhise may still have been saved to a file.

I have an object, of which I know that it can throw a recoverable exception within it's destructor, then I need nested try blocks. Otherwise (which in my case is the rule) I do not need nested try.

Martin

Disclaimer:
Having said that most objects are not excected to raise an exception in destruction => there are valid exception, and one has to know them, and act on them.

Here we have a big problem if the program is a web app using FastCGI. In this case, what we do?

MD.

Just one word, BRAVO!

MD.

Martin, I can't disagree more. I find it completely ludicrous that if obj1.free raises an exception you should just trow the baby out with the bath water. If Obj1.Free raised an exception because a disk drive was unmounted... Inspect the log... Handle the exception... And fix the problem. Future instances will be handled and all is good. It is my belief that the notion of skipping exception handling and restarting an application is very bad practice.

if " obj1.free; " crashed, then the state of the app is unknown, anything that the app does from now on could cause more havoc. trying to free obj2 could destroy value-able user-data that otherwhise may still have been saved to a file.

LOL. If something inside Obj1.free HAD EXCEPTION handling you would catch the error and understand what caused it.

procedure TObj1.Free;
begin
  Disk1.Unmount; <<< The disk object is missing now b/c a network failure;
  Inherited Destroy;
end;
procedure TObj1.Free;
begin
  Try
    Disk1.Unmount;
  Except
    On E:Exception do begin
       // Some case stuff here
    end;
  end;
  Inherited Destroy;
end;

In my example... Your code would blow out an application. With my exception handling the app would continue on as normal with no problems.

Write as much info as you have to the servers log file => so you can fix the app.
Return a 500 server error; that's what they are there for.

For the rest I don't know FastCGI good enough. But it needs a way to tell the server that t need's to be reloaded => e.g under apache it is common that each apache process is reused a certain amount of times => If a bad crash has happened then this process needs to be replaced.

It's good that in apache it's threads (or at least last time I check3ed) and not threads => if it was threads, you needed to kill the whole server, with processes, you only replace one child process.

Martin

Had you only read to the end of my mail..... You are deliberately ignoring part of what I wrote and the blame me for the remainder being incomplete?

I clearly stated that there are cases where "free" can raise a valid exception, and that the developper has to know this cases, and handle them (with nested try blocks).

But I also wrote that (for me) the rule is that most "free" are not expected to do so (that may of course be totally different in your code).

Martin

With the exception of a possible memory leak in case Disk1.Unmount was supposed to free up some resources. And if your app is something that runs 24/7, that is a *huge* problem! Eventually you will run out of memory and the whole thing will come crumbling down (and maybe even cause more damage).

On Wed, Jul 7, 2010 at 8:37 AM, Graeme Geldenhuys
wrote:
>> procedure TObj1.Free;

Grame, no more caveats. If Unmount had memory that needed to be released then the same methodology stands. Disk1.Unmount was placed there to prove a point. That w/o handling Obj2.Free would never be called in context (the outstanding proposed example).

As far as *HUGE* problem. Having to restart my server would be huge.. Especially in high scale, high available production environments. Having a handle in place that says CATCH STUFF HERE like Save to SQL server (if your needing to write to disk). The example I introduced is a good one. Ignoring the exception will indeed cause a fatal error. Handling the exception is the best practice here.

Martin, I didn't get to the end of your email because I find that first major part - part and parcel of why Delphi failed as a language. It became unviable because exception handling and blow-outs. Which in turn were caused by sloppy ideologies such as the standing notion here... Which is to let the blow-out occur and move on. Garbage collection was the solution to this problem.

Andrew Brunner wrote

As far as *HUGE* problem.  Having to restart my server would be huge.. Especially in high scale, high available production environments.

You don't have to restart the server (hardware), only the application or service (app server software). As in a busy environment that could be troublesome too, but a lot less troublesome than the whole server (hardware) doing down because it ran out of memory. Get it? If not, I don't care at point... enough said on the topic.

Hello FPC-Pascal,

Wednesday, July 7, 2010, 3:48:13 PM, you wrote:

AB> Martin, I didn't get to the end of your email because I find that AB> first major part - part and parcel of why Delphi failed as a language. AB> It became unviable because exception handling and blow-outs. Which AB> in turn were caused by sloppy ideologies such as the standing notion AB> here... Which is to let the blow-out occur and move on. Garbage AB> collection was the solution to this problem.

It's impossible to write a dogma about exceptions, some must be handled, some could or could not be handled. Expected exceptions like a conversion string to number should be handled and processed in that way to recover from them (they are recoverable).

If an exception is recoverable (program state can be passed from unestable to stable again) it should be handled and processed. If the handling does not garantee the stability the best option is to save the information that can be safelly stored or processed and terminate the process as soon as possible. It's not a matter of ideology, is a matter of stability. Unexpected exceptions should terminate the program as soon as possible with trying to save as much as possible. Even in some processes an unexpected exception should revert the program to the previous stable state (if possible) and discard all information in memory to avoid the possibility of corrupted information.

Ok, I know :) I talked about the mem state! Let's continue...

Well... I think CGI gateway is a good way. If the real app (FCGI) have troubles, I can change the URL that the CGI (gateway) uses to call the real app and to point to another app (in other server, e.g.). What do you think about it?

MD.

Nobody said handling exceptions is bad practice!
I understand that there are cases that have nothing to do when an exception (unexpectedly) is generated. We can not foresee everything.

MD.

Exactly.

I just wonder how you do in web apps that are not CGI. As I said earlier in an email here:
http://lists.freepascal.org/lists/fpc-pascal/2010-July/025902.html

...but I guess that is to be reviewed, perhaps, in another thread. ;-)

MD.

Well... I think CGI gateway is a good way. If the real app (FCGI) have troubles, I can change the URL that the CGI (gateway) uses to call the real app and to point to another app (in other server, e.g.). What do you think about it?

I think it would be worth discussing what can be done or at least start thinking about getting the memory manager not to blow-out any more. I really don't want to re-start server software just because a few apps (embedded in the service application) crash.

Has anyone with experience with developing the memory manager been following? What about a separate discussion on application stabilization after an unhanded exception? That would be a necessity for at least my platform which is compiled with FPC - and certainly FCLWeb generated back-end apps too.

I think is a good idea.
Do you uses FastCGI?

MD.

Hello FPC-Pascal,

Wednesday, July 7, 2010, 4:49:03 PM, you wrote:

MD> Exactly.
MD> I just wonder how you do in web apps that are not CGI. As I said MD> earlier in an email here:
MD> http://lists.freepascal.org/lists/fpc-pascal/2010-July/025902.html MD> ...but I guess that is to be reviewed, perhaps, in another thread. ;-)

I don't "do" :) because I do not have almost any experience in the web server world, but I'm quite sure you can let the exception scale up to the main process, which should end due unexpected exception and the watchdog restart it :-? In the other hand a simple "halt" should terminate the fastcgi application and this should force the web server to reload; or maybe fastcgi applications are modules ?

Yes, you can use "halt" to stop webserver, but is not good. If exists others apps running in the same webserver, you will stop them too! FastCGI would be a separated app, running in memory. I think it may be a module too, but I would not do that.
if you have an application that works separately from the web server, then it is easier to manage it (at least by hand, but I want to know how I do that automatically).

MD.

Indeed, it will drag the whole system down. That said (on linux, at least) when the program chomps too much memory, it'll get killed by the oom-killer anyway, so you're not much better off ignoring the exception. You can probably write a simple script to restart the app, as a fallback, after it sends an email to the admin containing the error message.

Henry

In our previous episode, Andrew Brunner said:

Well, first, there is the question if there is something to be fixed. (I myself think that plugging a few holes here and there once a quarter is might be worth avoiding the GC overhead and complexity. To be honest I never understood what all the fuzz about GC is about).

But anyway, the question has been asked before, and I then archived the responses here:

http://wiki.freepascal.org/garbage_collection

For the specific case of web applications, there are other possibilities, since in general, one could try to exploit the knowledge that anything allocated during a request can be deallocated. Like the old mark/heap

Of course somehow we would have to manage two heapmgrs then (for sessions and requests one each), and avoid polution the session heapmgr with request blocks. (e.g. by modifying ansistring etc helpers to avoid copy-on-write semantics)

One could even regularly stream the session(s) over IPC to another addressspace (or flush to DB) once in a while and reset the session space too.

Has anyone with experience with developing the memory manager been following?

Not likely, since I haven't seen any insights in this thread.

What about a separate discussion on application stabilization after an unhanded exception?

No system is safe for that. Even a managed .NET or Java app can NIL some variable in some unlikely codepath, and every subsequent request can raise a NIL exception, resulting in a effectively defunct app.

Moreover, GC only handles memory, not handles, DB connection etc.

A real solution is not availble.

That would be a necessity for at least my platform which is compiled with FPC - and certainly FCLWeb generated back-end apps too.

It is not a problem of FPC, but of all applications in general.

Fcl-web ? Why ?
1. All fcl-web backends handle exceptions that occur in the web-request handler.
2. There is never an excuse for not freeing allocated resources, also in cases of exceptions. Such leaks must be identified, and plugged. Not worked around. FPC offers the heaptrace unit exactly for this.

There is no way to allocate, use (and subsequently automatically free) memory associated with a web-request. One reason is simply string and class instance management, both of which inherently rely on a global heap manager without concept of 'context'.

Michael.

Wouldn't it be nice if we had a try..except..finally statement supported in FPC. All-in-one.

We do: it is called "try try"

try try
  // ...
except
  // ...
finally
  // ...

Okay, so maybe that's slightly nonstandard, but I'd rather use this awkwardness than add an even more awkward extra indentation level

~D.

I wonder where the *big* advantage of this "try..except..finally" is?

Looking at your code above there are 2 (simplified) cases.
1) you handle the exception in the except case, and it is ot re-risen
2) you re-raise it again
Variations of the 2 above:
3) a combination: sometimes handle,sometimes raise
4) the need to cover a new exception that may arise while handling the first exception

Lets look at them:
1)
   try
     DoDangerousStuff;
   except
     FixTheDamege
   end;
   DoYourFinalizationStuff;

Since any exception is handled, and no new exception is risen in the except block, the code after the except block is *always* executed. It does not need a "finally" statement.

2) re-raise the exception
   try
     DoDangerousStuff;
   except
     DoYourFinalizationStuff;
     LogTheExceptionAnd_Raise_ItAgain
   end;
   DoYourFinalizationStuff;

In case of an exception, the 2nd "DoYourFinalizationStuff;" is not called. To avoid code duplication, you would have to put the code into a local subroutine.

But yes, in this ase, an extra finally may be slighly more readable

3 and 4 are handled in the same way as 2.

Additionally, if your finalization code is only "FreeAndNil(SomeVar)", then you can add it in and after the exception block, and even allow it to run twice. The 2nd run does no damage.

my 2 cents
Martin