Timer - alternative implementation to DoEvents

Question

I am new to C#, and I needed to use a timer for a small application that I use for monitoring a piece of hardware. I found some reference code for timer, but it uses DoEvents(). Since, I run the timer for a long time, sometimes days and hours, I started getting stack overflow. I now understand that DoEvents() is causing this, and it is something most people recommend using. What feature would you recommend I use in place of DoEvents instead to setup my timer?

My code :

private void BeginMonitoringClick()  { 
{
    myTimer.Tick += new EventHandler(TimerEventProcessor); // myTimer declared elsewhere
    myTimer.Interval = 2000;
    myTimer.Start();

    while(!exitFlag)
    { 
        Application.DoEvents(); 
    }
}

private void TimerEventProcessor(Object myObject, EventArgs myEventArgs){
    // Talk to hardware, see if it is doing OK
}

Answer 1

Your StackOverflowException is possibly caused by repeated presses of the button. That would cause the BeginMonitoringClick() method to be called recursively, which would eventually overflow the stack.

Without seeing the rest of the code, it's impossible to know for sure. In code where DoEvents() is used once, it's not uncommon to see it used all over the place. You could have similar bugs elsewhere, each contributing to the problem.

Personally, I wish Microsoft had never included the DoEvents() method, nor its siblings Control.Refresh() and Control.Update(). Their use can lead to exactly this kind of re-entrancy based bugs, and I've never seen a situation where their use was actually required. There's always a more appropriate solution.

As others have noted, to fix the specific bug in this code, you should be able to just change your code by removing the loop:

private void BeginMonitoringClick()  { 
{
    myTimer.Tick += TimerEventProcessor; // myTimer declared elsewhere
    myTimer.Interval = 2000;
    myTimer.Start();
}

private void TimerEventProcessor(object myObject, EventArgs e) {
    // Talk to hardware, see if it is doing OK
}

It's not clear in your code example what the exitFlag variable was used for, so I can't explain an alternative for that. But if it was intended to be used to suspend/terminate the monitoring of your hardware, all you actually need to do is call myTimer.Stop() later, when you want that to happen.

Answer 2

I'd recommend you use a background thread.

   var tokenSource = new CancellationTokenSource();
        var token = tokenSource.Token;

        var backgroundTask = Task.Factory.StartNew( () => {

            while(!token.IsCancellationRequested)
            {
                Thread.Sleep(2000);
                // do work here
            }

        } , token );


        // save your tokenSource somewhere then you can cancel the thread whenever you are done with it.
        tokenSource.Cancel();