Jump to content
Ian Branch

caFree & ,Free??

Recommended Posts

Hi Team,

I have had the following construct in service for a loooong time without issue..

{code}

....

....

MyForm := TMyForm.Create(self);

try

with MyForm do

begin

...

....

....

end;

finally

MyForm.Free;

end;

{code}

 

in MyForm I have..

{code}

...

...

procedure TMyForm.FormClose(Sender: TObject; var Action: TCloseAction);
begin
Action := caFree;
end;

{code}

Is this construct OK?

 

Suddenly, I am starting to get 'EUseAfterFreeError's indicating the MyForm.Free;

My immediate thought is that the .free from the calling form is not finding something to Free because the called form has already done a caFree.

If this is the case, why has it just started?

 

Thoughts??

 

Regards & TIA,

Ian

Edited by Ian Branch

Share this post


Link to post
Guest

look... basically the rules is:

procedure TfrmFormMain.Button2Click(Sender: TObject);
var
  xNewFormInstance: TfrmFormSecond;
begin
  // xNewFormInstance := TfrmFormSecond.Create(Application); // the "Application" is responsable to destroy this object!
  //
  // xNewFormInstance := TfrmFormSecond.Create( <<another object>> ); // the "<<Another Object>>" is responsable to destroy this object!
  //
  // my preference is:
  xNewFormInstance := TfrmFormSecond.Create(nil); // "YOU" is responsable to destroy this object!
  //
  // xNewFormInstance := TfrmFormSecond.Create(Self);        // "ItSelf = Object" is responsable to auto-destroy itself on "OnClose" event!
  try
    xNewFormInstance.ShowModal;
    xNewFormInstance.Show; { normally, you use "OnClose" with "Action = caFree" }
  finally
    // if not using "Action := caFree" on "OnClose" event, or "xxxx.Create(Application)"
    xNewFormInstance.Free;
    xNewFormInstance := nil;
    // or
    FreeAndNil(xNewFormInstance);
  end;
end;

procedure TfrmFormSecond.FormClose(Sender: TObject; var Action: TCloseAction);
begin
  // Action := caFree; // using "xxx.Create( Self )", else, dont use it!
end;

 

NOTE: Some objects (OWNER), can "DONT" release all sub-objects created, like: Objects add in a "TList" (or similar) for example! Then, is necessary, that you "release" all sub-objects before release the "OWNER"

 

hug

Edited by Guest

Share this post


Link to post

Hi...:classic_cool:

Quote

Is this construct OK?

...No. It contains "with". :classic_tongue: "with" is like 90s...

Why use many developers the "with"...old school? Everybody knows by now that there can be problems with this...TRect (example) or no resolution of variables during debugging. :classic_huh:

Edited by haentschman
  • Like 2

Share this post


Link to post
Guest

Laziness, makes the hunter turn the hunt!
... and the lion thanks the Lord for another day's meal!

Share this post


Link to post
2 hours ago, Ian Branch said:

Is this construct OK?

Yes, it is perfectly safe. Using Action=caFree in the Form's OnClose event will call the Form's Release() method, which is a *delayed* destruction.  A CM_RELEASE message is posted to the Form's window. The Form is not actually destroyed until that message is processed at a later time. If the Form/window is destroyed before that happens, the message is discarded.

2 hours ago, Ian Branch said:

Suddenly, I am starting to get 'EUseAfterFreeError's indicating the MyForm.Free;

Then something else in your project is freeing the Form prematurely. It is not your OnClose handler doing it.  Run your code in the debugger and put a breakpoint in the Form's destructor or OnDestroy event, and look at the call stack to track down where the Form is being destroyed.

 

Share this post


Link to post
1 hour ago, emailx45 said:

look... basically the rules is:

It is perfectly safe to call Free() on a Form (or any other TComponent descendant) that has an Owner assigned.  The Form/component will simply remove itself from its Owner's internal list, so that the Owner will not try to destroy it a 2nd time when the Owner is destroyed.

Share this post


Link to post

I have a project that runs in OSX only,  Just moving it to Delphi 10.4.1 (I've seen the occasional memory exception when I close [Order Detail] - but I can't recreate it)

 

Is the nil/freeandnil the best way to deal with nested modal forms in a Firemonkey/OSX? All of the [forms] are modal and are created / freed in the same way.

 

TIA

 

Trevor

 

 

 

 

                | [Customer Search] -- [Add Customer] -- [Postcode Search] (for account record)
                |
Main -- [Order]-| [Postcode Search] (for del address)
                |
                |
                |                | [Part Search]-- [Add Part]
                | [Order Detail] |
                                 | [Add Part]


var
  f: TModalForm
begin
    f  := TModalForm.Create(nil);
    f.var := 100;
    try
      SetFormPos( f );
      if f.ShowModal = mrOk then 
        begin
        // do stuff ;
        // callingForm.var := f.var
        end;
    finally
      SaveFormPos( f );
      FreeAndNil(f);
    end;
end;

 

Share this post


Link to post
1 hour ago, TrevorS said:

I have a project that runs in OSX only,  Just moving it to Delphi 10.4.1 (I've seen the occasional memory exception when I close [Order Detail] - but I can't recreate it)

ShowModal is not supported on Android and it is not recommended on iOS, and if you trust documentations it should be safe to use on macOS. Issue is most likely caused by some other code, not your general form handling code that you have here. 

 

http://docwiki.embarcadero.com/Libraries/Sydney/en/FMX.Forms.TCommonCustomForm.ShowModal

 

1 hour ago, TrevorS said:

Is the nil/freeandnil the best way to deal with nested modal forms in a Firemonkey/OSX? All of the [forms] are modal and are created / freed in the same way.

Your form handling is mostly correct. I would remove SaveFormPos from finally block because if something goes wrong you most likely don't want to save your form position. Also, if SaveFormPos can raise an exception, you will leak the form because FreeAndNil will not be called in that case. Also I would put f.var assignment inside try... block, because if that assignment raises and exception you will also leak form. And since form variable f is local variable and it is going out of scope right after you are releasing it, there is very little point in calling FreeAndNil, simple f.Free is enough.

Share this post


Link to post

I have made the changes you suggested - I moved the SaveFormPos to the OnClose event of each form.  

I did spot an extra call to SaveFormPos in one of the branches ... so that's gone.

 

Thank you very much for your help, it is most appreciated :classic_cool:

Share this post


Link to post
7 hours ago, TrevorS said:

Is the nil/freeandnil the best way to deal with nested modal forms in a Firemonkey/OSX? All of the [forms] are modal and are created / freed in the same way.

Delphi 10.4+ uses traditional object lifetime management on all platforms (no more ARC on Android/iOS).  So, any object you Create(), that does not have an Owner assigned, must be Free()'d.

Edited by Remy Lebeau

Share this post


Link to post
On 2/16/2021 at 9:20 AM, Remy Lebeau said:

Yes, it is perfectly safe. Using Action=caFree in the Form's OnClose event will call the Form's Release() method, which is a *delayed* destruction.  A CM_RELEASE message is posted to the Form's window. The Form is not actually destroyed until that message is processed at a later time.

 

As recently has been mentioned in another thread, it's only safe if the code inside try/finally do not occasionally (or indirectly) call Application.ProcessMessages after ShowModal:

MyForm := TMyForm.Create(...);
try
  MyForm.ShowModal;
  Application.ProcessMessages; // Or SomeAnotherForm.ShowModal, etc.
finally
  MyForm.Free;
end;

In this case the form may be freed via CM_RELEASE, and then - freed the second time via the code in finally block.

 

 

 

Edited by balabuev
  • Like 2

Share this post


Link to post

This is a REALLY BAD APPROACH. I don't care if you use with or not. But you don't create an object then say "with <object> do" and then refer to the object in a Finally to free it AFTER you ALREADY FREED the object inside of the Try block! But you don't know at this point because you already (and invisibly) delegated the form's lifetime to the form itself. BAAAAAAADDDDDD!

 

Here's what you could use instead:

// Do NOT override the Create to inject parameters into the form unless they're actually required. 
with TMyForm.Create(self_or_NIL);
try
   // Forms rarely if ever need Constructor injection. The most common misuse is passing DB-related details into 
   // forms that are designed to only work with one table or one specific query or stored proc. It's hardwired.
   // It just needs a key to do a lookup. And don't rely on the "current record" in another form!
   // Instead, use PROPERTY INJECTION to initialize properties in TMyForm here, eg., by passing in a key.
   // And yes, TMyForm is an OBJECT and you should access things inside of it using PROPERTIES!
   // The setter for this property could trigger a DB lookup that initializes other fields on the form. 
   UserName := 'Fred Flintstone'; 
   . . .
   // now show the form, and do NOT delegate lifetime to the form
   if (ShowModal = mrOK) then  // requires setting the Action param to caClose NOT caFree!!!
   begin
      // now retrieve values from properties in the form, if needed.
      // note that you can't do this if you use caFree since it's DESTROYED at this point!
      // which is why it's a Bad Idea to delegate lifetime to the form.
      // You're using a with statement here, so you can't set it to nil. And even if you weren't,
      // there's no way to know that the form killed itself.
      // Unfortunately, it will work often enough that you'll think the occasional exception is for other reasons.
      Result := PhoneNum;
      . . .
   end;
finally
   // NOW Free the form instance
   Free;
end;

This is a nice, solid pattern that will serve you well in ALL form interactions. The only excuse not to use it (and not use PROPERTIES inside of the form) is if you're being LAZY.

 

The use of with can be debated until the cows come home and it boils down to a religious debate; but at the end of the day it's really a personal preference as far as I'm concerned.

 

But the way you've done it is obviously error-prone and not something you'd want to do everywhere (for consistency).

 

See my CodeRage 9 talk for more on this topic:
https://www.youtube.com/watch?v=JgOhg2GbydQ

 

Edited by David Schwartz

Share this post


Link to post
Guest
  • I have never liked the use of "WITH" in my projects since I started contacting Delphi.

 

Here is my example of a possible option to get around the "Access Violation" exception when trying to "free a object owner of a HANDLE, like TForm" ...
We can verify if the "xForm HANDLE exists yet later a caFree or close for example", but, using "if xForm.HANDLE> 0 then ..." by default will be create a "HANDLE value for it", then, the documentation talk about use of "xForm.HandleAllocated" for this situation. Avoiding a creation of a "HANDLE" for it.

 

Quote

Vcl.Controls.TWinControl.HandleAllocated

Reports whether a screen object handle exists for the control.

Query HandleAllocated to find out if the control's underlying screen object has been generated.

If the screen object exists, HandleAllocated returns true. If the screen object does not exist, HandleAllocated returns false. Testing the Handle property of a control directly causes the window to be created if it does not already exist. Call the HandleAllocated method to determine whether a window exists without creating one as a side effect.

 

FormMain:

type
  TfrmFormMain = class(TForm)
    btn_Calls_SecondForm: TButton;
    btn_HowManyFormsLiveYet: TButton;
    procedure btn_Calls_SecondFormClick(Sender: TObject);
    procedure btn_HowManyFormsLiveYetClick(Sender: TObject);
  private
    { Private declarations }
  public
    { Public declarations }
  end;

var
  frmFormMain: TfrmFormMain;

implementation

{$R *.dfm}

uses
  uFormSecond;

procedure TfrmFormMain.btn_Calls_SecondFormClick(Sender: TObject);
begin
  frmFormSecond := TfrmFormSecond.Create(nil);
  //
  try
    frmFormSecond.ShowModal;
    //
	// by Balabuev idea
    Application.ProcessMessages; // test some other way, like "send a message to close SecondForm"...
  finally
    // comment "if" line for get the "access violation" exception!!!
    if frmFormSecond.HandleAllocated then { not use xForm.HANDLE, because, if it dont exist, then, it will be created by default!!! - side-effects! }
	// your IDE stay stucked without "if" above - on Debug mode
      FreeAndNil(frmFormSecond);
  end;
end;

procedure TfrmFormMain.btn_HowManyFormsLiveYetClick(Sender: TObject);
var
  i     : integer;
  lForms: string;
begin
  lForms   := '';
  for i    := 0 to (Screen.FormCount - 1) do
    lForms := Format('%s%s'#10#13, [lForms, Screen.Forms[i].Name { or ToString if no-named } ]);
  //
  ShowMessage('Forms on memo:'#10#13 + lForms);
end;

initialization

ReportMemoryLeaksOnShutdown := true;

finalization

end.

 

FormSecond:

type
  TfrmFormSecond = class(TForm)
    btn_SecondForm_Close_Hide_procedure: TButton;
    btn_SecondForm_ModalResult_use: TButton;
    rdgrpCloseOptions: TRadioGroup;
    rdgrpModalResult: TRadioGroup;
    rdgrpCloseHideMinimizeForm: TRadioGroup;
    btn_FormsOnMemory: TButton;
    Label1: TLabel;
    procedure btn_SecondForm_Close_Hide_procedureClick(Sender: TObject);
    procedure FormClose(Sender: TObject; var Action: TCloseAction);
    procedure FormCloseQuery(Sender: TObject; var CanClose: Boolean);
    procedure btn_SecondForm_ModalResult_useClick(Sender: TObject);
    procedure btn_FormsOnMemoryClick(Sender: TObject);
    procedure FormKeyDown(Sender: TObject; var Key: Word; Shift: TShiftState);
    procedure FormCreate(Sender: TObject);
  private
    { Private declarations }
  public
    { Public declarations }
  end;

var
  frmFormSecond: TfrmFormSecond;

implementation

{$R *.dfm}

uses
  System.UITypes;

procedure TfrmFormSecond.FormCreate(Sender: TObject);
begin
  Caption := Format('%s - %s', [Caption, DateTimeToStr(now)]);
end;

procedure TfrmFormSecond.btn_FormsOnMemoryClick(Sender: TObject);
var
  i     : integer;
  lForms: string;
begin
  lForms   := '';
  for i    := 0 to (Screen.FormCount - 1) do
    lForms := Format('%s%s'#10#13, [lForms, Screen.Forms[i].Name { or ToString if no-named } ]);
  //
  ShowMessage('Forms on memo:'#10#13 + lForms);
end;

procedure TfrmFormSecond.btn_SecondForm_Close_Hide_procedureClick(Sender: TObject);
begin
  case rdgrpCloseHideMinimizeForm.ItemIndex of
    1:
      Self.Hide;
  else
    Self.Close;
  end;
end;

procedure TfrmFormSecond.btn_SecondForm_ModalResult_useClick(Sender: TObject);
begin
  Self.ModalResult := rdgrpModalResult.ItemIndex;
  (*
    case rdgrpModalResult.ItemIndex of
    1:
    Self.ModalResult := mrOk;
    2:
    Self.ModalResult := mrCancel;
    3:
    Self.ModalResult := mrAbort;
    4:
    Self.ModalResult := mrRetry;
    5:
    Self.ModalResult := mrIgnore;
    6:
    Self.ModalResult := mrYes;
    7:
    Self.ModalResult := mrNo;
    8:
    Self.ModalResult := mrClose;
    9:
    Self.ModalResult := mrHelp;
    10:
    Self.ModalResult := mrTryAgain;
    11:
    Self.ModalResult := mrContinue;
    12:
    Self.ModalResult := mrAll;
    13:
    Self.ModalResult := mrNoToAll;
    14:
    Self.ModalResult := mrYesToAll;
    else
    Self.ModalResult := mrNone;
    end;
  *)
end;

procedure TfrmFormSecond.FormCloseQuery(Sender: TObject; var CanClose: Boolean);
begin
  CanClose := true;
end;

procedure TfrmFormSecond.FormKeyDown(Sender: TObject; var Key: Word; Shift: TShiftState);
begin
  if (Key = VK_F1) and not(Self.Showing) then
    Self.Show;
end;

procedure TfrmFormSecond.FormClose(Sender: TObject; var Action: TCloseAction);
begin
  case rdgrpCloseOptions.ItemIndex of
    0:
      Action := caFree;
    1:
      Action := caHide;
    2:
      Action := caMinimize;
  else
    Action := caNone;
  end;
end;

end.

 

 

hug

Edited by Guest

Share this post


Link to post
On 2/16/2021 at 4:20 AM, Ian Branch said:

MyForm := TMyForm.Create(self);

try

with MyForm do

begin

...

....

....

end;

finally

MyForm.Free;

end;

 

in MyForm I have..

 

...

...

procedure TMyForm.FormClose(Sender: TObject; var Action: TCloseAction);
begin
Action := caFree;
end;

 

 

Is this construct OK?

No. It is not OK. You are trying to release the form twice.  Once explicitly by calling Free and once by setting on close Action to caFree.

 

This may work under some circumstances, like @balabuev mentioned caFree triggers posting CM_RELEASE message to the form window handle. If there is no code that will pump the message queue (like Application.ProcessMessages) between the time OnFormClose is called and your explicit call to Free, then calling Free will actually destroy the form and along with it its window handle and message posted to that handle will be discarded because window no longer exists.

 

However, if CM_RELEASE message is processed and handled before your explicit Free call, form will already be destroyed at the point you call Free and hence the error you are seeing (presumably from EurekaLog)

 

On 2/16/2021 at 4:20 AM, Ian Branch said:

Suddenly, I am starting to get 'EUseAfterFreeError's indicating the MyForm.Free;

My immediate thought is that the .free from the calling form is not finding something to Free because the called form has already done a caFree.

If this is the case, why has it just started?

Because you have added some other code in between that is calling Application.ProcessMessages.

 

You have to choose one option. Either you will use caFree or you will use Free. Using both and hoping for the best is not good option even if it works, because it can be easily broken, which you now have first hand experience.

 

If you need to retrieve some value from the form after it is closed, then you shouldn't use caFree anyway because you may access dead form - again depending on the other surrounding code. Another reason to explicitly Free form is that this makes memory management more obvious. You create form, do something with it, and then you release it.

 

  • Like 1

Share this post


Link to post
Guest

for all that ... my preferrnce is always use NIL (on create) and FREE (on final) explicitly...(not Action:= caFree; on forms )..... or FreeAndNil for more easy way...  for sure, with some verifying added.

if any error, it should be mine... not of language!

 

of course, the bug can stay on language, too! but this is another and long history beyond my responsability.

 

hug

Edited by Guest

Share this post


Link to post
8 hours ago, balabuev said:

As recently has been mentioned in another thread, it's only safe if the code inside try/finally do not occasionally (or indirectly) call Application.ProcessMessages after ShowModal

This is only dangerous if the code is using BOTH Action=caFree with Free() together, which makes no sense to do.  Use only one or the other, not both.

 

Share this post


Link to post
2 hours ago, emailx45 said:
  • Here is my example of a possible option to get around the "Access Violation" exception when trying to "free a object owner of a HANDLE, like TForm" ...

This example is ABSOLUTELY WRONG and MUST NOT be used.  Yes, you can use HandleAllocated() to check if a UI control's HWND exists without creating a new HWND, but you can't use HandleAllocated() to determine if the UI object itself exists.  If a Form uses Action=caFree in its OnClose event to request itself be destroyed, and the resulting CM_RELEASE message gets processed (as this example is doing), the Form object will be destroyed, and any subsequent attempt to access its members, including HandleAllocated() (as this code is doing) will be undefined behavior, and likely will result in a crash.

 

Edited by Remy Lebeau
  • Like 1

Share this post


Link to post
8 minutes ago, Remy Lebeau said:

This is only dangerous if the code is using BOTH Action=caFree with Free() together, which makes no sense to do.  Use only one or the other, not both.

 

And that is exactly the situation OP has encountered here. I guess you missed that part.

  • Like 1

Share this post


Link to post

Hi Team,

FWIW, I have gotten rid of all the caFrees in my code.

Sticking with the Try.....Finally xxxx.Free end construct.

No more issues.

Thank you all for your input/feedback.

Ian

Share this post


Link to post
Guest

@Remy Lebeau

read help about 

xxxx.HandleAllocated

read the post ... 

4 hours ago, emailx45 said:

for sure, with some verifying added.

ADD a ( not(frmSecondForm=nil)) ...

 

you try my sample in action?

 

I bet that no!

 

then... nothing to say.

 

hug

Edited by Guest

Share this post


Link to post
9 hours ago, emailx45 said:

@Remy Lebeau

read help about 


xxxx.HandleAllocated

read the post ... 

I suggest that you read once again what @Remy Lebeau posted. You example is horrible and no amount of dancing around it will ever make it work properly. 

 

When you run it, and it works... that will only be an illusion caused by undefined behavior and a touch of luck.

  • Like 1

Share this post


Link to post
Guest

you are not beautiful either, so we are tied.

 

And, the clear cooperativism can also be noticed.

 

Anyway, licking other people's lives can bring serious infections to the soul.

 

IF YOU LIKE BEAUTY LOOK THAT and Marco analisis:

Protecting Two Objects: How NOT to Write the Code --> or more objects, OF COURSE!

...

https://blogs.embarcadero.com/try-finally-blocks-for-protecting-multiple-resources-in-delphi/  == NOTHING IT'S REALLY SECURITY AND IS RIGHT FOR SURE

Quote

Protecting Two Objects: A Tale of Three Solutions

This long introduction brings us to the point of this blog post. There are at least 3 different correct solution for the issue of protecting two resources in the same code block, as I just mentioned. Here are the three solutions in an image I “borrowed” from one of our RAD Studio R&D architects, Bruneau Babet.

image.thumb.png.546ce01245ccf4bc93242ccd4f1e1e34.png

 

Provided they are all correct in terms of proper resource management in all scenarios, which are the advantages and disadvantages of these 3 solutions? What is important to consider is that the 2 resources could be 3, or 4 or half a dozen. 

Which One to Pick?

The first solution with the nested try blocks fairly clean but more verbose (more lines of code) and has additional nesting that could become an annoyance with multiple resources. Also, there is a runtime cost associated with try blocks, clearly limited but not zero. 

The second solution has the least amount of lines of code code and the least amount of runtime cost. The only additional is setting A2 to nil. The code remains readable also with many resources. However, the code is “unbalanced” and it might be slightly confusing.

The third solution goes in the same direction, but it adds one extra technically useless assignment to nil (for A1), offering the advantage of being cleaner and more balanced, and likely more readable after all.

So what’s the best solution? This is really hard to tell,. I personally mostly used #1 in my books, but at Embarcadero we tend to prefer the cleaner and faster ones (that is, #2 or #3) for library code. Interested in your opinions, of course.

 

hug

Edited by Guest

Share this post


Link to post
18 hours ago, emailx45 said:

@Remy Lebeau

read help about 


xxxx.HandleAllocated

I don't need to read the help on it, I already know exactly what it is and how it works, thank you.

Quote

read the post ... 

ADD a ( not(frmSecondForm=nil)) ...

That is written simpler as (frmSecondForm <> nil).

But either way, checking for nil STILL won't solve the issue, because the frmSecondForm pointer is NOT being set to nil when the TfrmSecondForm object is destroyed, so ANY check for nil after destruction will NOT work in your example.  The code you have shown earlier will STILL have undefined behavior even with the nil check added:

procedure TfrmFormMain.btn_Calls_SecondFormClick(Sender: TObject);
begin
  frmFormSecond := TfrmFormSecond.Create(nil);
  try
    frmFormSecond.ShowModal;
    Application.ProcessMessages; // <-- object destroyed here if TfrmFormSecond.rdgrpCloseOptions.ItemIndex = 0
  finally
    // if object is destroyed above, UB happens below because frmFormSecond is a dangling pointer!

    if frmFormSecond.HandleAllocated then
      FreeAndNil(frmFormSecond);
    // or
    if (frmFormSecond <> nil) and frmFormSecond.HandleAllocated then
      FreeAndNil(frmFormSecond);
  end;
end;

And before you say it, yes you could fix that specific issue by setting the global frmSecondForm pointer to nil during destruction, like in the Form's OnDestroy event, eg:

procedure TfrmSecondForm.FormDestroy(Sender: TObject);
begin
  frmSecondForm := nil;
end;

procedure TfrmFormMain.btn_Calls_SecondFormClick(Sender: TObject);
begin
  frmSecondForm := TfrmFormSecond.Create(nil);
  try
    frmSecondForm.ShowModal;
    Application.ProcessMessages; // <-- object destroyed here if TfrmFormSecond.rdgrpCloseOptions.ItemIndex = 0
  finally
    if frmSecondForm.HandleAllocated then // <-- if object is destroyed above, UB here because frmSecondForm is now nil!
      FreeAndNil(frmSecondForm);
    // or
    if (frmSecondForm <> nil) and frmSecondForm.HandleAllocated then // <-- no more UB here!
      FreeAndNil(frmSecondForm);
  end;
end;

But, that won't solve the issue if someone decides NOT to use the global pointer to begin with, eg:

procedure TfrmFormMain.btn_Calls_SecondFormClick(Sender: TObject);
var
  frm: TfrmFormSecond;
begin
  frm := TfrmFormSecond.Create(nil);
  try
    frm.ShowModal;
    Application.ProcessMessages; // <-- object destroyed here if TfrmFormSecond.rdgrpCloseOptions.ItemIndex = 0
  finally
    // if object is destroyed above, UB happens below because frm is a dangling pointer!

    if frm.HandleAllocated then
      FreeAndNil(frm);
    // or
    if (frm <> nil) and frm.HandleAllocated then
      FreeAndNil(frm);
  end;
end;

To solve that, you would have to do something radical along the lines of the following:

procedure TfrmSecondForm.FormDestroy(Sender: TObject);
type
  PTfrmSecondForm = ^TfrmSecondForm;
begin
  PTfrmSecondForm(Tag)^ := nil;
end;

procedure TfrmFormMain.btn_Calls_SecondFormClick(Sender: TObject);
var
  frm: TfrmFormSecond;
begin
  frm := TfrmFormSecond.Create(nil);
  try
    frm.Tag := NativeInt(@frm);
    frm.ShowModal;
    Application.ProcessMessages; // <-- object destroyed here if TfrmFormSecond.rdgrpCloseOptions.ItemIndex = 0
  finally
    if frm.HandleAllocated then // <-- if object destroyed above, UB here because frm is nil!
      FreeAndNil(frm);
    // or
    if (frm <> nil) and frm.HandleAllocated then // <-- no more UB here!
      FreeAndNil(frm);
  end;
end;

But, no matter what, using HandleAllocated() to determine whether the Form object needs to be destroyed or not is just plain WRONG.  Object lifetime and window lifetime are two completely different things.  If you Create() a Form (or any object), you need to Free() it when you are done using it, if that is not already being done elsewhere (by an Owner, by an OnClose event, etc).

Quote

you try my sample in action?

 

I bet that no!

No, because I don't need to run it (and besides, I can't run it, as I don't have a working compiler installed at the moment).  Just by looking at the code, my many years of experience and deep understanding of the VCL's inner workings tell me EXACTLY how this code will behave.

 

Edited by Remy Lebeau
  • Like 1

Share this post


Link to post
On 4/5/2021 at 7:58 AM, Guest said:

Provided they are all correct in terms of proper resource management in all scenarios, which are the advantages and disadvantages of these 3 solutions? What is important to consider is that the 2 resources could be 3, or 4 or half a dozen. 

Marco is enumerating a few ways of approaching a common situation. Which you choose is up to you, based in large part on surrounding contextual stuff you might know about in that situation  that someone else would not.

 

You're asking something like, "What is the best way to introduce a person to another person in terms of what you say?"

 

Are they two friends of yours?

Are they strangers to you?

Is one a dignitary?

Is one the opposite sex and you have to follow certain rules?

Are they possibly antagonistic towards each other (eg, a drug dealer vs. a police officer)?

Is one a family member or someone you are concerned about for some reason?

Do you have an agenda in introducing them? 

 

You seem to be asking for something to say that will work in 100% of situations. It's not possible without sometimes sounding like an idiot.

 

You need to adapt your interactions to the situation at hand, just like you would employ different water management policies in a rain forest as opposed to a desert, although you'd use the same tools in both cases.

 

 

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×