TCastleView.Stop called from FINALIZATION section

I had a really weird SIGSEGV in my project and I wasn’t able to understand why as I were cleaning and nulling everything correctly. In a long GDB session I’ve found that view’s Stop method is called from a finalization section! That conflicts my cleaning because I’m using a global object I create in initalization and free in finalization that is initialized in the view’s Start method and finalized in the view’s Stop method.

I’m aware my design isn’t the best (it’s an ugly hack just for testing) but anyway I think that view’s Stop method shouldn’t be called after program termination. Why isn’t it called from TApplication.Terminate? Why is it delayed to the finalization section?

This is like TForm’s event OnDestroy: It is guaranteed that it’s called before the program finished.

On the contrary, you can test. Running the attached example shows that after the main begin .. end. ends, the form gets OnHide and OnDestroy events.

terminate_finalization.zip (140.7 KB)

What happens and why:

Application.Terminate merely sets a flag to terminate as soon as possible. This is true both for Application singleton used by LCL (from LCL Forms unit) and for Application singleton from CastleWindow unit.

When using CastleWindow: the finalization section of CastleWindow actually stops any view (old name: “UI state”) we were in, then frees window and application.

When using LCL it is similar, the finalization section of Forms does FreeThenNil(Application).

The above behavior is standard. LCL applications (unrelated to CGE) also behave lke this.

Why?

  • We cannot stop and free things right when Application.Terminate is called, as it’s often called from callbacks/methods of a form/view. Freeing everything from Application.Terminate would require extra-careful usage of Application.Terminate.

  • The main program, of both LCL application and CGE application, doesn’t have any other explicit call to “free everything now”. LCL depends on finalization of Forms. CGE depends on finalization of CastleWindow.

Solutions in your case:

  1. You can make your Stop method “resilient” to the fact that it’s called from finalization. If it accesses something else, that may be already freed at this point, then check is it nil before.

  2. Alternatively: you can get the behavior you want by modifying the auto-generated CGE program file, xxx_standalone.dpr. By default it’s really trivial:

    begin
      Application.MainWindow.OpenAndRun;
    end.
    

    It is OK to edit it, if you know what you’re doing, you can edit it. (By “know what you’re doing” I mean: remember that running "“Code → Regenerate Program” from CGE will overwrite it, so you will have to reapply your change; but we don’t do this regeneration automatically ever, now).

    Just set the current view to nil , to make sure it is stopped at this point:

    begin
      Application.MainWindow.OpenAndRun;
      Application.MainWindow.Container.View := nil;
    end.
    
  3. Alternatively: you can add the Application.MainWindow.Container.View := nil; call before you free your global object. I understand you have a line like this right now:

    finalization
      FreeAndNil(SomeGlobalObjectUsedByViewsStop);
    end;
    

    So just change it to this:

    finalization
       // make sure all views are stopped now, 
      // so that we don't call any TXxxView.Stop after SomeGlobalObjectUsedByViewsStop is freed
      Application.MainWindow.Container.View := nil;
      FreeAndNil(SomeGlobalObjectUsedByViewsStop);
    end;
    
1 Like

Then I misunderstood it.

I take note of the ways to fix it. I think 3. will be my favourite.

1 Like

You probably have this figured out by now but I had the same issue. I found using Application.MainWindow.CloseQuery was a good way to intercept app shutdown before the finalizations. I am in agreement that finalizations are a dangerous place to clean up since order is arbitrary.

I will just add that I still don’t recommend OnCloseQuery for this purpose, for reasons explained in comment in Where to intercept closing application? - #3 by michalis :slight_smile: I know it works in your project, but in general

  • OnCloseQuery is a way to “ask for confirmation before closing form (e.g. to save some document)”, not “a place to do cleanup”.

  • Stop is a place to do cleanup, and if you want Stop of your view to happen before some finalization, then use Application.MainWindow.Container.View := nil , as shown in this thread (point 3 above, which I understand the OP followed) or Where to intercept closing application? - #3 by michalis .

I suffered crashes on close for years. Spent so long trying to figure out. Even crashes closing the vanilla client/server demo when data is flowing. Now they are all solved. Maybe if you used threads more, you would realize the danger of doing too much in finalization. This solved it all…

procedure TViewMain.StopServerAndExit;
 begin
  ShowNotification( 'Stop water flow threads.' );
  StopWaterFlowThreads;
  StopServer;
  Application.MainWindow.Close;
 end;

procedure TViewMain.CloseQuery(AContainer: TCastleContainer);
 begin
   StopServerAndExit;
 end;

I feel kinda stupid for not managing to realize until a couple days ago that stop was being called from finalization! I got too rusty using other languages (which all suck). Best to clean up your own garbage haha.

If there was something like an OnBeforeClose callback, that would be ideal to get the threads shutdown before finalizations. In the meantime CloseQuery is that callback. I can’t just check nils as the the threads walk multiple arrays of 100,000+ singles doing constant addition at 60+ tiles/second to insure the objects don’t get deleted out from under it. In the VCL world they offered something like that but memory fails. I am happy with how things work now, you don’t need to do add anything.

Doing things in OnCloseQuery is executing them in the same thread as you would as in finalization. I don’t think you achieve anything by doing things in OnCloseQuery versus in “finalization after making sure that views have stopped by Application.MainWindow.Container.View := nil”.

And since OnCloseQuery is not going to run always when window is closed by explicit Window.Close ( Where to intercept closing application? - #3 by michalis ), it’s still not the best place to make “cleanup that must be executed always” :slight_smile:

I realize that things are difficult when using threads, and client/server, but this doesn’t change the above recommendation.

  • If you have threads, you must cleanup the data correctly.

  • If you client/server in threads, you may need to also wait for those threads.

  • I realize it’s not easy (and why Indy can make either memory leaks or crashes, as documented in 3.4. Note: There are known memory leaks when using Indy).

  • Still, if you want to make a “cleanup that always executes”, doing it in Stop of the view is what I recommend. This is our equivalent of OnBeforeClose callback that you mention.

    And then if you need to do some “global cleanup” in some finalization, that depends that views are stopped, do it after calling Application.MainWindow.Container.View := nil – as this makes sure that all views are stopped. This will work regardless of how the window got closed (by Alt+F4, by Window.Close, by Application.Terminate etc.).

1 Like