4

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);
        }
JoseOrtiz3
  • 1,785
  • 17
  • 28
  • 1
    Try removing the event first, `updateStuff.Elapsed -= updateStuff_Elapsed;` then proceed with the disposal code. This should probably be done in the `FormClosing` method though, and just call `updateStuff.Dispose` in the `Dispose` method. – Ron Beyer May 13 '15 at 20:39
  • I tried calling `updateStuff.Elapsed -= updateStuff_Elapsed` before stopping, closing, disposing the timer, no luck. My forms application does not appear to have a FormClosing method, is that a method of the form class from which form1 is subclassed? – JoseOrtiz3 May 13 '15 at 20:51
  • You have to add an override for it. In your form class, start typing `override` and you'll see the `OnFormClosing` in the list of methods, select it to create an override and put the code in there. Alternatively you can create an event in the constructor for the `FormClosing` event, but you should override it instead of using internal events like that. – Ron Beyer May 13 '15 at 20:52
  • IT WORKS!! I just typed in override, found the OnFormClosing event, pressed tab and visual studio took care of everything. Wow. Thanks Ron. – JoseOrtiz3 May 13 '15 at 21:00
  • 1
    Do not embed answers, particularly those based on someone else's, into your question. If you have your own answer and is sufficiently different to others then post it as an answer below –  May 13 '15 at 21:20

3 Answers3

3

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]));
          }
    }
}
Ňuf
  • 6,027
  • 2
  • 23
  • 26
0

To expand on what Ron and Marc said, The solution was to override the OnFormClosing method of the class Form from which Form1 is subclassed:

    protected override void OnFormClosing(FormClosingEventArgs e)
    {
        //It appears it needs everything here to be turned off...amazing.
        updateStuff.Elapsed -= updateStuff_Elapsed;
        updateStuff.AutoReset = false;
        updateStuff.Stop();
        updateStuff.Dispose();
        base.OnFormClosing(e);
    }

The other solution is to put the timer on the same thread as the user interface, as Nuf stated.

JoseOrtiz3
  • 1,785
  • 17
  • 28
  • Disposing Timer in OnFormClosing method IMHO doesn't solve the problem. At best, it makes problem more unlikely to happen (or in other words it increases probability that all queued event handler calls will finish before Form is disposed), but it doesn't guarantee proper functionality. – Ňuf May 13 '15 at 22:45
-1

Handling the dispose event is too late (which is during the garbage collection). You need to perform the cleanup process on the FormClosing event.

Marc Johnston
  • 1,276
  • 1
  • 7
  • 16