Problem closing forms..

[Ian Branch 128]

Hi Team,
    D10.4, 32bit App, Win 10.
    I am calling a sub form from a sub form as follows..

      //
  MyForm := TMyForm.Create(self);
  MyForm.sFormID := '3';    //  Pass a key value.
  MyForm.Show;
  //

It all works fine.
I close the sub-sub form in the FormClose event as follows..

    //
  Action := caFree;
  //

I close the sub-form the same way..

    //
  Action := caFree;
  //

Problem - When I close the sub form I get an Access violation error.
If I remove the "Action = caFree" from the sub form it closes without issue.
Can somebody please enlighten me as to what is going on and how to resolve it?
  
Regards & TIA,
Ian 

[Guest]

Can you reproduce the behaviour in small example and test it on older versions of Delphi ?

 

[aehimself 399]

4 hours ago, Ian Branch said:

 

      //
  MyForm := TMyForm.Create(self);
  MyForm.sFormID := '3';    //  Pass a key value.
  MyForm.Show;
  //

 

You are creating the form with the owner of your main form. When you close your main form therefore (as it owns your subform) it attempts to free it. But since it is already done in the onClose event (Action := caFree) it is trying to free a non-existing object resulting an access violation.

Try to create your subform like

 

TMyForm.Create(nil);

 

Or don't use caFree in the onClose event.

 

You should consider using

MyForm := TMyForm.Create(nil);
Try
 MyForm.sFormID := '3';
 MyForm.ShowModal;
Finally
 MyForm.Free;
End;

if applicable.

[Ian Branch 128]

Hi Team,

Thank you for your inputs.

Application structure.  MainForm => Form1 => Form2.

I changed (self) to (nil).  Same issue.

I don't want Form2 Modal as it needs to show while the User continues in Form1.

I created a bare bones app with the same structure.

I didn't get the issue when closing Form1 after closing Form2. 🙂

In my main App I created a new, empty Form2.  I got the error again when I tried to close the Form1 after closing Form2. 😞

The error still happens if I disable the Action := caFree in Form2, but not if I disable it in Form1.

The error generated is per the attached image.  'DBiLogViewer.exe' is the name of the App.

 

Regards,

Ian

 

[Guest]

Let me explain this:

A is your sub-form and B is sub-sub-from

 

1) A do create B and take ownership of it

2) if A.Close called, then caFree will trigger self destroy and will notify the owner of A.

3) if A.Free called then it will call Close first then continue to free like above.

4) in both case above Free will loop over all child's ( forms and controls and any B's) and free them one by one.

5) If A own B, then it will call B.Free from the destructor of A, means A might be already destroyed something inside the destructor while your code in B is trying to communicate with A, this might be a reason for that.

6) Keep in mid that B.Free is not called form the destructor but from the ancestor destructor using the inherited destructor, means the destructor of A most like destroyed most if not all of A fields and childs.

7)...

 

I think you got the idea, why i asked if it is repeatable on older Delphi version, because unless you are doing something wrong in the B destructor there is a small chance that may be Delphi 10.4 have a bug somewhere.

 

Anyway, aehimself suggested two solution and i don't agree with them both as they will restrain you from building the GUI you want, so my suggestion is this

In the destructor of A, loop over all the child control and find all B's ( and other forms ) and simply call Close on them, do then as first step in A destructor and make sure the last line is the inherited.

The suggestion might remove the exception and make it fine and safe to use, if it did help then you are violating something in B, something you shouldn't be doing in the first place.

 

hope that help.

[Ian Branch 128]

Hi Kas Ob,

I tried the same App in D10.3.3 with the same result.

I have continued with Create(nil) when creating the forms.

I sort of follow what you are saying above.

Using your terminology, normally I would expect B to be closed before A.

As a test I closed A while B was still open, I got the error. 😞

It is definitely something specific with my App.  As indicated previously, I created a dummy App, only components were buttons to call the next form, with the same hierarchy and there was no issue.

For now I will just not use 'Action := Free;' in the close of A.

 

Regards & Tks,

Ian

[Guest]

@Ian Branch you did read my text but seems you missed the solution i suggested,

42 minutes ago, Kas Ob. said:

In the destructor of A, loop over all the child control and find all B's ( and other forms ) and simply call Close on them, do then as first step in A destructor and make sure the last line is the inherited.

That is it, even better make it call and just call it form A destructor, as first thing in that call.

Do you know how to do it?

[Ian Branch 128]

Hi Kas Ob,

Regrettably, no. 😞

 

Regards,

Ian

Edited July 26, 2020 by Ian Branch

[Uwe Raabe 2064]

1 hour ago, Ian Branch said:

I created a bare bones app with the same structure.

I didn't get the issue when closing Form1 after closing Form2. 🙂

In my main App I created a new, empty Form2.  I got the error again when I tried to close the Form1 after closing Form2. 😞

So it works with the small example, but fails in your main app? In that case there is something in your code you didn't show us.

 

BTW, you indeed create a Form2 with Form1 as its owner, but as soon as you destroy Form2 it removes itself from Form1. So that is most likely not causing the problem.

 

Just to make sure: In your example you name both forms MyForm. I hope that are different variables?

[Guest]

Try this

procedure CloseAllChildForms(aOwner: TCustomForm);
var
  i: Integer;
  lOwner: TComponent absolute aOwner;
begin
  if aOwner is TCustomForm then
    if lOwner.ComponentCount > 0 then
      for i := 0 to lOwner.ComponentCount - 1 do
        if lOwner.Components[i] is TCustomForm then
          TCustomForm(lOwner.Components[i]).Close;
end;
//   or
procedure CloseAllChildForms(aOwner: TCustomForm);
var
  i: Integer;
begin
  if aOwner is TCustomForm then
    if TComponent(aOwner).ComponentCount > 0 then
      for i := 0 to TComponent(aOwner).ComponentCount - 1 do
        if TComponent(aOwner).Components[i] is TCustomForm then
          TCustomForm(TComponent(aOwner).Components[i]).Close;
end;

and call it from the destructor or the close event of your A ( sub-form), also to be safe as your design problem may be from somewhere else than A, but same concept B is trying to access something that had been freed, so put it in every form that might has childs!

Quote

procedure TAForm.FormDestroy(Sender: TObject);
begin
  CloseAllChildForms(Self);
end;

or

Quote

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

Just find the right place that fix your exception.

[Ian Branch 128]

Hi Uwe,

Yes, they had different names.

 

Kas Ob,

Thank you.  I will digest and have a play.

Will respond later with my results.

 

Regards,

Ian

[Ian Branch 128]

Hi Team,

Found it!  I progressively removed components/code from Form A until the issue disappeared.

Turns out to be a component from a well know component library supplier.  I will test the same component in a basic test App to confirm and let them know.

Thank you for your guidance and assistance.  I have learnt a couple of things from you.  Much appreciated.

Regards,

Ian

[Remy Lebeau 1436]

Setting Action=caFree in the OnClose event is a DEFERRED destruction.  It does not free the Form right away, it postpones the actual destruction until control flow returns to the calling thread's message loop, THEN the Form is freed when it is safe to do so (ie, no other messages are being processed).  So it doesn't matter if the Form is owned or not.  If the Form is owned, and the Owner destroys the Form before the caFree request is processed, then the request will simply get ignored since there is nothing to Free anymore.

[Ian Branch 128]

Hi Remy,

Now that is enlightening.  Thank you.

So, if I don't put 'Action = caFree' in the OnClose event, when does the from get destroyed.

Or, to ask in another direction, how can I ensure the form is destroyed when it is closed?

 

Regards & TIA,

Ian

[Remy Lebeau 1436]

1 hour ago, Ian Branch said:

So, if I don't put 'Action = caFree' in the OnClose event, when does the from get destroyed.

If it is owned, it is destroyed when its Owner is destroyed.  Otherwise, it is leaked and not destroyed at all.  When the process exits, the OS will reclaim the leaked memory.

Quote

how can I ensure the form is destroyed when it is closed?

Using Action=caFree in the OnClose event is the correct way to do that.  Whether or not the Form has an Owner is irrelevant.  An owned object can be destroyed at any time, even before its Owner is destroyed.  If the object is destroyed first, it will simply remove itself from its Owner so that that Owner does not try to destroy it again.

Edited July 28, 2020 by Remy Lebeau

[Ian Branch 128]

Hi Remy,

Ahhh.  Whew!  That's what I have been doing.

Thank you for the education.

 

Regards,

Ian

[aehimself 399]

17 hours ago, Remy Lebeau said:

An owned object can be destroyed at any time, even before its Owner is destroyed.  If the object is destroyed first, it will simply remove itself from its Owner so that that Owner does not try to destroy it again.

I swear I had "random" access violations when an application was closed which was caused by freeing something owned by the form... then, when the form was closed it tried to free it again.

Now, on Delphi 10.4 I simply can NOT reproduce the issue, no matter how hard I try.

 

Checking the destructor code of TComponent:

destructor TComponent.Destroy;
begin
  Destroying;
  RemoveFreeNotifications;
  DestroyComponents;
  if FOwner <> nil then FOwner.RemoveComponent(Self); // THIS LINE
  FObservers.Free;
  inherited Destroy;
end;

Was THIS LINE always there? If I'm not mistaken the bug mentioned happened on Delphi 10 Seattle, before we made the upgrade... is it possible that it was implemented inbetween?

If not, how exactly it can be reproduced?!

 

Parents of mentioned components were frames, if it makes any difference

[Uwe Raabe 2064]

1 hour ago, aehimself said:

Was THIS LINE always there?

In the moment I am able to track it down to Delphi 7, but I am pretty confident that it exists since Delphi 1.

[Remy Lebeau 1436]

1 hour ago, aehimself said:

I swear I had "random" access violations when an application was closed which was caused by freeing something owned by the form... then, when the form was closed it tried to free it again.

Then they likely weren't being owned properly to begin with.

Quote

Was THIS LINE always there?

Yes.

Quote

is it possible that it was implemented inbetween?

No.

Quote

Parents of mentioned components were frames, if it makes any difference

In fact, that even makes it more complex, because a Parent also destroys its child controls when itself is destroyed, outside of TComponent ownership.

destructor TWinControl.Destroy;
var
  I: Integer;
  Instance: TControl;
begin
  ...
  I := ControlCount;
  while I <> 0 do
  begin
    Instance := Controls[I - 1];
    Remove(Instance);
    Instance.Destroy; // <-- HERE
    I := ControlCount;
  end;
  ...
end;

 

16 minutes ago, Uwe Raabe said:

In the moment I am able to track it down to Delphi 7, but I am pretty confident that it exists since Delphi 1.

I have VCL sources going back to Delphi/BCB 5.  Here is the TComponent destructor in D5:

destructor TComponent.Destroy;
var
  I: Integer;
begin
  Destroying;
  if FFreeNotifies <> nil then
  begin
    for I := FFreeNotifies.Count - 1 downto 0 do
    begin
      TComponent(FFreeNotifies[I]).Notification(Self, opRemove);
      if FFreeNotifies = nil then Break;
    end;
    FFreeNotifies.Free;
    FFreeNotifies := nil;
  end;
  DestroyComponents;
  if FOwner <> nil then FOwner.RemoveComponent(Self); // <-- HERE
  inherited Destroy;
end;

IIRC, there was not much in the way of sweeping changes between D1 to D5, so I agree with Uwe that TComponent ownership likely dates back all the way to D1, when the VCL was first introduced.

Edited July 28, 2020 by Remy Lebeau

[Anders Melander 1815]

1 hour ago, Remy Lebeau said:

TComponent ownership likely dates back all the way to D1

Indeed. Here's the Delphi 1 version:

destructor TComponent.Destroy;
begin
  Destroying;
  DestroyComponents;
  if FOwner <> nil then FOwner.RemoveComponent(Self);
  DisposeStr(FName);
end;

 

It's been a while since I looked through the Delphi 1 sources. Classes.pas is only 3916 lines long. It's just sooo cute

Edited July 28, 2020 by Anders Melander

[David Schwartz 430]

Forms are objects. They behave just like objects. And IMHO, they should be treated just like any other objects.

 

All TObjects have a Constructor and Destructor. 

 

What's normally done is:
 

myObj := TMyObj.Create;
try
  myObj.abc := 123; // initialize things
  . . .
  myObj.DoSomething;
  rslt1 := myObj.GetResult; // get stuff out of the object
  . . .
finally
  myObj.Free;
end;

TForms are TObjects, and they also have:

* OnCreate and OnDestroy. 

* OnShow and OnHide.

* OnClose

 

Things you create in the OnCreate method should be freed in OnDestroy.

 

Things you create and enable in OnShow often need to be freed / disabled in OnHide.

 

OnClose is a quirk. It's useful for some purposes, but it can often cause more problems than it's worth.

 

Forms can be created and destroyed automatically, so they're persistent throughout the life of the application. You just use normal properties and methods to access things on the form (although most people don't bother with defining properties to interact with forms).

 

For typical dynamic form use, this approach can be used:
 

myForm := TMyForm.Create(NIL);
try        // OnCreate is called here
  myForm.abc := 123; // initialize things
  . . .
  myForm.Show;                        // OnShow ... do stuff ... OnHide
  // do some stuff here before calling Free
  . . .
  -- or use ShowModal --
  if (myForm.ShowModal = mrOK) then   // OnShow ... do stuff ... OnHide
  begin
    rslt1 := myForm.GetResult; // get stuff out of the object
    . . .
  end;
finally
  myForm.Free;   // OnDestroy is called here
end;

This is really clean. The structure is just like any other object whether you need to add parameters to initialize the object (form) before use or get result data afterwards.

 

The only times I've run into issues is when I rely on the OnClose method with caFree. People don't like using 'with' because of its implicit references. Well, relying on OnClose with caFree presents similar implicit actions, and can be equally problematic.

 

If you rely on caFree in the OnClose handler, and you decide to get some result data from the form later, you have a lot more work to do because the object myForm pointed to has been freed after ShowModal returns.

Edited July 29, 2020 by David Schwartz

[Remy Lebeau 1436]

58 minutes ago, David Schwartz said:

For typical dynamic form use, this approach can be used

Show() is non-blocking, so your example would reach myForm.Free() before the Form's window is actually shown to the user.

1 hour ago, David Schwartz said:

If you rely on caFree in the OnClose handler, and you decide to get some result data from the form later, you have a lot more work to do because the object myForm pointed to has been freed after ShowModal returns. 

It is perfectly safe to use caFree with a modal Form.  The Form object remains alive until after ShowModal() exits and control returns to the main message loop, THEN the Form is destroyed.  If you want to get results from the Form, you just have to do so immediately after ShowModal() exits, which is the best time to get the results anyway.  If you need the results at a later time, save them somewhere that you can get them back when needed.

 

So, you would either:

 

-auto-create a Form and then Show()/Hide() it when needed. Let the TApplication handle destroying the Form during app shutdown.

 

- Create() and Show() the Form, then Free() it when you don't need it anymore, or use OnClose to caFree it.

 

- Create() and ShowModal() the Form, and then Free() it (directly or via caFree) after ShowModal() has exited.

[David Schwartz 430]

3 minutes ago, Remy Lebeau said:

Show() is non-blocking, so your example would reach myForm.Free() before the Form's window is actually shown to the user.

Yeah, I was just looking at that.... I've changed it to be more clear.

Edited July 29, 2020 by David Schwartz