Multiple timer elapsed issue

Question

I have a timer as defined below. The timer executes a long running task. The problem i have is when the timer is running the interval has elapsed again and another timer execution starts i.e _timer_Elapsed. How can i get the timer to execute one interval after the timer has finished. The way it is now there could be multiple timer executions happening at once and that causes all sorts of issues with my code.

protected override void OnStart(string[] args)
        {

           _timer = new System.Timers.Timer();
           _timer.AutoReset = false;
           _timer.Interval = (Convert.ToInt32(ConfigurationManager.AppSettings["CheckInterval"]));
           _timer.Elapsed += new System.Timers.ElapsedEventHandler(_timer_Elapsed);
           _timer.Enabled = true;
           _timer.Start(); // Start timer 

        }


 public static void _timer_Elapsed(object sender, System.Timers.ElapsedEventArgs e)
        {
            try
            {
                _timer.Interval = (Convert.ToInt32(ConfigurationManager.AppSettings["CheckInterval"])); 

                BLLGenericResult SystemCheckItems_Result = ServiceItemOperations.CheckItems(); // Long Running Task

            }
            catch (Exception ex)
            {
                // Exception handling...
            }
            finally
            {
                _timer.Start();
            }
        }

That's not possible, using AutoReset = false ensures that the timer cannot fire again unless you explicitly re-enable it. Which you may well do the wrong way, like re-enabling it at the start of the Elapsed event handler instead of the end. Or not properly implementing the OnStop() method. — Hans Passant, Apr 13 '14 at 14:13
@HansPassant yeah - i was reading that also - let me put up my _Timer_Elapsed method and see what you think — user1438082, Apr 13 '14 at 14:15
Or...you called the function OnStart again and a New timer is started in addition to the previous one which is still running. — Steve Wellens, Apr 13 '14 at 14:16
@SteveWellens I dont think so - OnStart is only called once - On windows Service startup — user1438082, Apr 13 '14 at 14:19
Move the Interval assignment to the bottom as well. Yet another System.Timers.Timer quirk. — Hans Passant, Apr 13 '14 at 14:20
@HansPassant looks good hans do you want to add as a solution - is this a c# bug ? — user1438082, Apr 13 '14 at 14:26

score 3 · Accepted Answer · answered Apr 13 '14 at 14:59

Let's properly write this up. It is the Interval assignment that is getting you into trouble. A sample program that demonstrates the issue:

using System;

class Program {
    static System.Timers.Timer timer;
    static void Main(string[] args) {
        timer = new System.Timers.Timer();
        timer.AutoReset = false;
        timer.Interval = 1000;
        timer.Elapsed += timer_Elapsed;
        timer.Start();
        Console.ReadLine();
    }

    static void timer_Elapsed(object sender, System.Timers.ElapsedEventArgs e) {
        timer.Interval = 1000;
        Console.WriteLine("Tick");
    }
}

Output:

Tick
Tick
Tick
...

Remove the Interval assignment in the Elapsed event handler to see the difference.

So, in other words, even though the AutoReset property is false, just assigning the Interval property is enough to get the timer to restart. Big surprise of course, nobody ever sees that coming. And it plays havoc on your program because you assign the Interval property early, before you start doing the heavy lifting. So the Elapsed event will be raised again, on another thread, while your previous one hasn't completed running yet. That's a threading bug waiting to happen, particularly if your Interval value is too small. You'll need to assign it later, do so inside the finally block. Good thing that you made the Interval too small, very hard to diagnose later.

is this a c# bug ?

The C# language is off the hook, this is a .NET Framework bug. System.Timers.Timer is in general just not a very good class, it has the usability of a chainsaw without a safety switch. It was meant to make the System.Threading.Timer class more usable, in particular solving a problem with the habit of it being garbage collected too soon. One problem solved, but three new problems added. This goes back to .NET 1.0, a framework version with training wheels. They just can't fix it anymore, too much existing code will break. You really are better off with the System.Threading.Timer, just make sure to keep the object referenced in a variable. Like you already do.

The detail you have gone to here is impressive - thank you very much. — user1438082, Apr 13 '14 at 15:02

score 0 · Answer 2 · answered Jul 28 '17 at 03:57

0

A 2017 update:

There is a new tool that can be useful with these kinds of problems.

await Task.Delay(TimeSpan.FromSeconds(x));

answered Jul 28 '17 at 03:57

tymtam

31,798
8
86
126

Multiple timer elapsed issue

2 Answers2

Linked