Hi there,

If my class constructor looks like this:

constructor TMyClass.Create(fn: string);
begin
  sl := TStringList.Create;
  try
    fs := TFileStream.Create(fn, fmOpenRead);
  except
    self.Destroy;
  end;
end;

I create the objec like: MyInstance :=
TMyClass.Create('AnNonExistentFile'); An exception occured, I can ensure that:

1. there is no memory leak.
2. the MyInstance variable is assigned *nil*?

Thanks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus.freepascal.org/pipermail/lazarus/attachments/20130302/d25feaa8/attachment.html>

I assume FPC behaves like Delphi, so there's no leak. Actually, you don't even need to explicitly call Destroy; when an exception occurs in a constructor the destructor is called automatically.

2. the MyInstance variable is assigned nil?

No, it's not initialized.

Thanks

Regards,
Fl?vio

No, this is a bad idea. If an exception occurs inside the constructor the destructor will be called automatically. I don't even want to think about what bad things could happen if you call Destroy inside the constructor...

There won't be a memory leak.

2. the MyInstance variable is assigned *nil*?

If you want to ensure that MyInstance is Nil you need to do it like this:

=== code begin ===

try
   MyInstance := TMyClass.Create('AnNonExistentFile');
except
   MyInstance := Nil;
end;

=== code end ===

Regards,
Sven

But you can write:

constructor TMyClass.Create(fn: string);
var fs: TFileStream;
begin
   sl := TStringList.Create;
   try
     fs := TFileStream.Create(fn, fmOpenRead);
   except
     self:= nil;
   end;
end;

which gives no memory leak, (provided your destructor frees the stringlist sl) and also ensures MyInstance is assigned a nil value.

This will definitely cause a memory leak, because the FPC code does not know that an exception occurred in the constructor (you catch it with Except), and hence will not call the destructor.

Heap dump by heaptrc unit
27 memory blocks allocated : 2067/2104
25 memory blocks freed : 1947/1984
2 unfreed memory blocks : 120
True heap size : 425984
True free heap : 425568
Should be : 425608
Call trace for block $00007FEF86F892C0 size 104
$000000000041205C
$0000000000411D4A
$0000000000400283 line 15 of tm.pp
$00000000004004CC line 30 of tm.pp
$000000000040018F
Call trace for block $00007FEF86F810C0 size 16
$000000000041205C
$00000000004001F7 line 14 of tm.pp
$00000000004004CC line 30 of tm.pp
$000000000040018F

Also, setting Self to nil is nonsense.

If you do FreeAndNil(Self) instead, it will work.

But I would seriously warn against doing this kind of thing.

The idea of exceptions is exactly to let the exception propagate to callers, so they know something is wrong. With the code as you propose, MyInstance is nil, without any notification whatsoever that something went wrong or what exactly went wrong.

Michael.

When an exception occurs in Create, the assignment will never take place. That's why MyInstance should be set to Nil *before* calling the constructor. This also applies to memory leak protection of multiple temporary objects:

inst1 := nil;
inst2 := nil;
...
try
   inst1 := T1.Create;
   inst2 := T2.Create;
   ...
finally
   inst1.Free;
   inst2.Free;
   ...
end;

The Free does no harm, even if the instances never were created, when the variables are nil'ed before the try block.

DoDi

Michael Van Canneyt wrote

This will definitely cause a memory leak, because the FPC code does not know that an exception occurred in the constructor (you catch it with Except), and hence will not call the destructor.

Thanks for the correction and explanation.

Howard

(...)

Wow, this is unexpected. I always thougth that by definition a constructor would never return nil - so Self was a "by-value" parameter, even inside a constructor.
Is this by design?
I never saw the point of allowing assign to Self,...

-Fl?vio