locked
Foreground thread termination question RRS feed

  • Question

  • We have a C# application that for some reason sometimes when closing certain parts of the "OnClosing" function are not executed.  This function is overriden in the main form of our application. It's almost as if the foreground thread abnormally terminates.  Is this possible, if so what can I do to ensure a graceful termination?   We are not aborting the application anywhere else.   I'm also wondering if the reason why we're seeing this is because "OnClosing" is obsolete? 

    I have read that it is recommended to use "OnFormClosing" instead.     

    Thanks! 

    Tuesday, October 23, 2012 11:32 PM

Answers

  • OnFormClosing() seems to be called in more scenarios when the application is closed.

    OnClosing() is not even called if the Application.Exit() methods are called.

    What kind of code do you have in the form Closing event? I assume it is code that is kind of critical to execute when the form closes?

    If any thread calls the Environment.FailFast() method, then there is not much you can do. This FailFast() method is intended to do exactly that, close the application fast.

    You can have a look at the following event handlers which may help you determine what you are missing. COMMENT or UNCOMMENT the code as needed.

    using System;
    using System.ComponentModel;
    using System.Runtime.ConstrainedExecution;
    using System.Windows.Forms;
    
    namespace WindowsFormsApplication1 {
    
        public partial class Form1 : Form {
    
            // A finalizer type to "know" when the finalizer thread is collecting this form as garbage.
            readonly FormFinalizer _Finalizer;
    
            public Form1() {
    
                InitializeComponent();
    
                // A finalizer type to "know" when the finalizer thread is collecting this form as garbage.
                _Finalizer = new FormFinalizer();
            }
    
            protected override void OnClick(EventArgs e) {
    
                base.OnClick(e);
                
                // Option 1 - NEITHER will be called
                //Environment.FailFast("Testing");
    
                // Option 2 - Only OnFormClosing() is called
                // Application.Exit();
    
                // Option 3 - Only OnFormClosing() is called
                //CancelEventArgs CancelArgs = new CancelEventArgs();
                //Application.Exit(CancelArgs);
    
                // Option 4 - ALL 3 are called
                //Close();
            }
    
            protected override void OnFormClosing(FormClosingEventArgs e) {
    
                base.OnFormClosing(e);
    
                Console.Beep();
            }
    
            protected override void OnClosing(CancelEventArgs e) {
    
                base.OnClosing(e);
    
                Console.Beep();
            }
        }
    
        class FormFinalizer : CriticalFinalizerObject {
    
            ~FormFinalizer() {
    
                Console.Beep();
            }
        }
    }
    

    The Application and AppDomain also provide the following "closing" events, but they will also not execute if the FailFast() method was called:

    using System;
    using System.Runtime.ExceptionServices;
    using System.Windows.Forms;
    
    namespace WindowsFormsApplication1 {
    
        static class Program {
    
            /// <summary>
            /// The main entry point for the application.
            /// </summary>
            [STAThread]
            static void Main() {
    
                AppDomain.CurrentDomain.DomainUnload += new EventHandler(CurrentDomain_DomainUnload);
    
                AppDomain.CurrentDomain.ProcessExit += new EventHandler(CurrentDomain_ProcessExit);
    
                AppDomain.CurrentDomain.FirstChanceException += new EventHandler<FirstChanceExceptionEventArgs>(CurrentDomain_FirstChanceException);
    
                AppDomain.CurrentDomain.UnhandledException += new UnhandledExceptionEventHandler(CurrentDomain_UnhandledException);
    
                Application.EnableVisualStyles();
                Application.SetCompatibleTextRenderingDefault(false);
                Application.Run(new Form1());
            }
    
            static void CurrentDomain_UnhandledException(object sender, UnhandledExceptionEventArgs e) {
    
                /* an exception was not handled by the application. See e.IsTerminating. */
            }
    
            static void CurrentDomain_FirstChanceException(object sender, FirstChanceExceptionEventArgs e) {
    
                /* an exception occurred and this is where you know about it first, whether it will be handled or unhandled */
            }
    
            static void CurrentDomain_ProcessExit(object sender, EventArgs e) {
    
                /* the application process is closing */
            }
    
            static void CurrentDomain_DomainUnload(object sender, EventArgs e) {
    
                /* an appdomain (probably the only) is closing */
            }
        }
    }

    Wednesday, October 24, 2012 8:46 AM
  • First, you should not be overriding OnAnything unless you are creating user controls.

    You should not handle an XXX event if you inherit from the class where it is defined. You should override the OnXXX method instead.

    • Edited by Louis.fr Wednesday, October 24, 2012 3:26 PM
    • Marked as answer by Bob ShenModerator Wednesday, November 7, 2012 4:40 AM
    Wednesday, October 24, 2012 3:26 PM
  • You should not handle an XXX event if you inherit from the class where it is defined. You should override the OnXXX method instead.

    You should override "OnXXX" if you want to change when and/or how the event is triggered.  Examples include suppressing the event (all of the time, or just in certain situations) changing the arguments that are passed to the event, etc.  You also may need to override "OnXXX" if it's particularly important that you execute some code and ensure it always runs before all event handlers or after all event handlers (which shouldn't be something you need to do).

    You should attach a handler to the event (even in an inheriting class) if you just want to execute something when that event occurs.

    • Proposed as answer by Dan Randolph Wednesday, October 24, 2012 5:21 PM
    • Marked as answer by Bob ShenModerator Wednesday, November 7, 2012 4:25 AM
    Wednesday, October 24, 2012 5:17 PM

All replies

  • First, you should not be overriding OnAnything unless you are creating user controls.

    You should implement the standard FormClosing event handler as Closing is obsolete since .NET 2.0.

    See this link for details and example: http://msdn.microsoft.com/en-us/library/system.windows.forms.form.formclosing.aspx


    Dan Randolph - My Code Samples List

    • Proposed as answer by servy42 Wednesday, October 24, 2012 3:05 PM
    Wednesday, October 24, 2012 5:00 AM
  • OnFormClosing() seems to be called in more scenarios when the application is closed.

    OnClosing() is not even called if the Application.Exit() methods are called.

    What kind of code do you have in the form Closing event? I assume it is code that is kind of critical to execute when the form closes?

    If any thread calls the Environment.FailFast() method, then there is not much you can do. This FailFast() method is intended to do exactly that, close the application fast.

    You can have a look at the following event handlers which may help you determine what you are missing. COMMENT or UNCOMMENT the code as needed.

    using System;
    using System.ComponentModel;
    using System.Runtime.ConstrainedExecution;
    using System.Windows.Forms;
    
    namespace WindowsFormsApplication1 {
    
        public partial class Form1 : Form {
    
            // A finalizer type to "know" when the finalizer thread is collecting this form as garbage.
            readonly FormFinalizer _Finalizer;
    
            public Form1() {
    
                InitializeComponent();
    
                // A finalizer type to "know" when the finalizer thread is collecting this form as garbage.
                _Finalizer = new FormFinalizer();
            }
    
            protected override void OnClick(EventArgs e) {
    
                base.OnClick(e);
                
                // Option 1 - NEITHER will be called
                //Environment.FailFast("Testing");
    
                // Option 2 - Only OnFormClosing() is called
                // Application.Exit();
    
                // Option 3 - Only OnFormClosing() is called
                //CancelEventArgs CancelArgs = new CancelEventArgs();
                //Application.Exit(CancelArgs);
    
                // Option 4 - ALL 3 are called
                //Close();
            }
    
            protected override void OnFormClosing(FormClosingEventArgs e) {
    
                base.OnFormClosing(e);
    
                Console.Beep();
            }
    
            protected override void OnClosing(CancelEventArgs e) {
    
                base.OnClosing(e);
    
                Console.Beep();
            }
        }
    
        class FormFinalizer : CriticalFinalizerObject {
    
            ~FormFinalizer() {
    
                Console.Beep();
            }
        }
    }
    

    The Application and AppDomain also provide the following "closing" events, but they will also not execute if the FailFast() method was called:

    using System;
    using System.Runtime.ExceptionServices;
    using System.Windows.Forms;
    
    namespace WindowsFormsApplication1 {
    
        static class Program {
    
            /// <summary>
            /// The main entry point for the application.
            /// </summary>
            [STAThread]
            static void Main() {
    
                AppDomain.CurrentDomain.DomainUnload += new EventHandler(CurrentDomain_DomainUnload);
    
                AppDomain.CurrentDomain.ProcessExit += new EventHandler(CurrentDomain_ProcessExit);
    
                AppDomain.CurrentDomain.FirstChanceException += new EventHandler<FirstChanceExceptionEventArgs>(CurrentDomain_FirstChanceException);
    
                AppDomain.CurrentDomain.UnhandledException += new UnhandledExceptionEventHandler(CurrentDomain_UnhandledException);
    
                Application.EnableVisualStyles();
                Application.SetCompatibleTextRenderingDefault(false);
                Application.Run(new Form1());
            }
    
            static void CurrentDomain_UnhandledException(object sender, UnhandledExceptionEventArgs e) {
    
                /* an exception was not handled by the application. See e.IsTerminating. */
            }
    
            static void CurrentDomain_FirstChanceException(object sender, FirstChanceExceptionEventArgs e) {
    
                /* an exception occurred and this is where you know about it first, whether it will be handled or unhandled */
            }
    
            static void CurrentDomain_ProcessExit(object sender, EventArgs e) {
    
                /* the application process is closing */
            }
    
            static void CurrentDomain_DomainUnload(object sender, EventArgs e) {
    
                /* an appdomain (probably the only) is closing */
            }
        }
    }

    Wednesday, October 24, 2012 8:46 AM
  • First, you should not be overriding OnAnything unless you are creating user controls.

    You should not handle an XXX event if you inherit from the class where it is defined. You should override the OnXXX method instead.

    • Edited by Louis.fr Wednesday, October 24, 2012 3:26 PM
    • Marked as answer by Bob ShenModerator Wednesday, November 7, 2012 4:40 AM
    Wednesday, October 24, 2012 3:26 PM
  • You should not handle an XXX event if you inherit from the class where it is defined. You should override the OnXXX method instead.
    In Windows forms, your formX instance inherits from Form where all these events are defined. Thus your statement is not accurate unless I am not understanding it.

    Dan Randolph - My Code Samples List

    Wednesday, October 24, 2012 3:54 PM
  • You should not handle an XXX event if you inherit from the class where it is defined. You should override the OnXXX method instead.
    In Windows forms, your formX instance inherits from Form where all these events are defined. Thus your statement is not accurate unless I am not understanding it.

    Dan Randolph - My Code Samples List


    In Windows forms, your formX instance inherits from Form. Therefore you should override OnFormClosing instead of subscribing to the FormClosing event.
    Wednesday, October 24, 2012 4:22 PM
  • In Windows forms, your formX instance inherits from Form. Therefore you should override OnFormClosing instead of subscribing to the FormClosing event.

    Your statement is contrary to how the Visual Studio Designer works.

    http://msdn.microsoft.com/en-US/library/0y0987sc%28v=vs.100%29.aspx

    If you are right, then why does Microsoft set up the designer to do it the other way?


    Dan Randolph - My Code Samples List

    Wednesday, October 24, 2012 4:35 PM
  • You should not handle an XXX event if you inherit from the class where it is defined. You should override the OnXXX method instead.

    You should override "OnXXX" if you want to change when and/or how the event is triggered.  Examples include suppressing the event (all of the time, or just in certain situations) changing the arguments that are passed to the event, etc.  You also may need to override "OnXXX" if it's particularly important that you execute some code and ensure it always runs before all event handlers or after all event handlers (which shouldn't be something you need to do).

    You should attach a handler to the event (even in an inheriting class) if you just want to execute something when that event occurs.

    • Proposed as answer by Dan Randolph Wednesday, October 24, 2012 5:21 PM
    • Marked as answer by Bob ShenModerator Wednesday, November 7, 2012 4:25 AM
    Wednesday, October 24, 2012 5:17 PM
  • "You should attach a handler to the event (even in an inheriting class) if you just want to execute something when that event occurs."

    Actually the documentation says otherwise:

    "The OnFormClosing method also allows derived classes to handle the event without attaching a delegate. This is the preferred technique for handling the event in a derived class." - http://msdn.microsoft.com/en-us/library/system.windows.forms.form.onformclosing.aspx

    And it says the same thing for pretty much all other OnXXX methods in WinForms since the days of .NET 1.0...

    Wednesday, October 24, 2012 5:24 PM
  • Do you know why that practice is suggested Mike?

    As far as I can tell they will be functionally equivalent, with the exception that overriding the OnXXX methods could be done wrong and break all sorts of stuff (if you don't call the base method) whereas you can't really break anything attaching an event handler.  Sure, attaching an event handler might take a tiny bit longer, enough that I *might* care for something like MouseMove that will be called a lot, but wouldn't even consider for something happening as infrequently as FormClosing.

    Wednesday, October 24, 2012 5:47 PM
  • "Do you know why that practice is suggested Mike?"

    Because it's more natural to override a virtual method instead of attaching an event? Because it's more efficient especially in WinForms where events aren't backed by fields but by an EventHandlerList?

    Anyway it doesn't really matter. The advice not to override OnClosing is pointless to begin with. If OnClosing is not called then neither the corresponding event will be raised and the OP problem is not solved.

    Wednesday, October 24, 2012 5:59 PM
  • Thanks everyone for your responses.    Let me do some more investigation on this one.   I was able to put more trace statements into the program and fortunately was able to reproduce.  Looks like maybe the foreground thread is not terminating in the middle of OnClosing.   But it's really strange that some code *seems* to be completely bypassed.   I'm going to have to do more investigation to come up with a better understanding of the problem.   Then I'll repost.    Thanks! 

     

    Wednesday, October 24, 2012 6:05 PM
  • Your statement is contrary to how the Visual Studio Designer works.

    Which part of the Visual Studio Designer are you talking about?

    The link you're proposing shows the handling of a Button event. The button is a control on the form. The form doesn't derive from it. You cannot override its OnClick method. Nothing to do with what I said.

    Even if that page had shown the handling of, for example, Form_Load, it wouldn't mean much. It's a page on how to consume events. Of course they'll show you what the title suggests. It doesn't mean it's the only or the best way.

    Here is an example overriding the OnPaint method instead of handling the Paint event: http://msdn.microsoft.com/en-us/library/cksxshce.aspx

    And look here: http://msdn.microsoft.com/en-us/library/system.windows.forms.control.onclick.aspx

    "The OnClick method also allows derived classes to handle the event without attaching a delegate. This is the preferred technique for handling the event in a derived class." (emphasis mine)

    Thursday, October 25, 2012 4:54 PM