Search code examples
c#multithreadingwinformstimerdispose

How to prevent Timer Elapsed event from occurring if forms application closes C#


I have a timer in a windows forms application (Visual C#) that is causing problems when I want to exit the application.

The timer is defined as a member of the form's class:

 partial class Form1 
     {
        //These are the members in question:
        internal ComACRServerLib.Channel channel;
        private System.Timers.Timer updateStuff;
     }

The timer is declared/constructed in the form application's constructor:

public Form1()
        {
            InitializeComponent();
            updateStuff = new System.Timers.Timer();
            updateStuff.Elapsed += new System.Timers.ElapsedEventHandler(updateStuff_Elapsed);
        }

The timer is started and configured with a push of a button:

 private void btnAcquire_Click(object sender, EventArgs e)
    {
        updateStuff.Interval = 100;
        updateStuff.Enabled = true;
        updateStuff.AutoReset = true;
        updateStuff.Start();
    }

When the timer elapses, it calls updateStuff_Elapsed, which gets information to be displayed with setText(there's some code to make sure the call to setText is thread-safe).

 private void updateStuff_Elapsed(object sender, System.Timers.ElapsedEventArgs e)
       {
              if (!channel.isOffline)
              {
                  object[] status = channel.GetACRCustom("P6144");
                  setText(System.Convert.ToString(status[0]));
              }
       }

 public delegate void setTextDelegate(string text);

 public void setText(string text)
       {
             if (this.lblTest.InvokeRequired == true)
             {
                  setTextDelegate d = new setTextDelegate(setText);
                  this.Invoke(d, new object[] { text });
             }
             else
             {
                  lblTest.Text = text;
             }
       }

On application exit I try to get rid of the timer and prevent it from firing again with the following:

protected override void Dispose(bool disposing)
        {

            if (disposing && (components != null))
            {
                updateStuff.AutoReset = false;
                updateStuff.Stop();
                updateStuff.Close();
                updateStuff.Dispose();

                components.Dispose();
            }
            base.Dispose(disposing);
        }

But if the timer is autorunning and I exit the program, I always get errors that the routine called by the Timer Elapsed event updateStuff_elapsedis attempting to use resources that have been disposed already! Even though I have tried my best to stop and destroy the timer before disposal occurs.

How do I stop the timer from firing when the application closes??

EDIT

I tried moving the Dispose code around to try to force the timer to close but no luck. I also tried using updateStuff.Elapsed -= updateStuff_Elapsed to remove the event call before stopping and disposing;

protected override void Dispose(bool disposing)
        {
            //now this code HAS to run always.
            updateStuff.Elapsed -= updateStuff_Elapsed;
            updateStuff.AutoReset = false;
            updateStuff.Stop();
            updateStuff.Close();
            updateStuff.Dispose();

            if (disposing && (components != null))
            {           
                components.Dispose();
            }
            base.Dispose(disposing);
        }

Solution

  • As stated in documentation for System.Timers.Timer, Timer's event handler calls are queued on ThreadPool thread. Therefore you must assume that event handler can be called multiple times at once or can be called after Timer is disabled. So event handler must be designed to handle these situations properly.

    First, set SynchronizingObject property of timer to instance of Form. This will marshall all calls of event handler to UI thread, so we don't need to bother with locking of form fields (we will always access everything from the same UI thread). With this property set, you also don't need to call this.Invoke(...) in setText method.

    public Form1()
    {
        updateStuff = new System.Timers.Timer();
        updateStuff.SynchronizingObject = this;
        ...
    }
    
    public void setText(string text)
    {
        lblTest.Text = text;
    }
    

    Then create flag, that will let you know, wheather timer has been disposed or not. Then simply check this flag in event handler:

    partial class Form1 
    {
        private bool Disposed;
        ....
    }
    
    protected override void Dispose(bool disposing)
    {
        if (disposing && (components != null))
        {
                updateStuff.Dispose();
                Disposed = true;
        }
    
        base.Dispose(disposing);
    }
    
    private void updateStuff_Elapsed(object sender, System.Timers.ElapsedEventArgs e)
    {
        if(!Disposed)
        {
              if (!channel.isOffline)
              {
                  object[] status = channel.GetACRCustom("P6144");
                  setText(System.Convert.ToString(status[0]));
              }
        }
    }