How to fix a memory issue with my timer loop

Question

I have a question about a timer loop I am performing in my C# winforms app. Every 50 seconds it's supposed to check the time on the system, and compare it to a user defined time. If that time matches the current time, perform the function, if not then return to the timer and wait another 50 seconds. I thought I had it working correctly but after leaving the application running overnight I came back and my CPU was at 99% and it was using 3.5gb memory. I checked with the timer disabled and the rest of the application runs fine so it's got to be something with the timer.

here is the timer code:

        public void day_timer()
    {
        System.Timers.Timer timer2 = new System.Timers.Timer();
        timer2.Elapsed += new ElapsedEventHandler(daily_Elapsed);
        timer2.Interval = 50000;
        timer2.Enabled = true;
        timer2.Start();
    }

and here is the elapsed code:

        private void daily_Elapsed(object sender, EventArgs e)
    {
        string time_rightmeow = DateTime.Now.ToString("hh:mm tt");
        if (time_rightmeow != Configuration.Day_time)
        {
            day_timer();
        }
        else
        {
            day_backup();
        }

    }

as you can see it's pretty simple, but I am wondering if there is a more efficient way to do this. I am assuming that the timer is not being disposed properly or something to that effect.

any Ideas?

Every time day_timer is called, you create a new timer. – Lee O. Nov 19 '13 at 01:26 — Lee O., Nov 19 '13 at 01:26

score 6 · Accepted Answer · answered Nov 19 '13 at 01:24

You don't have to start another timer again. The one you've set will fire Elapsed event every Interval milliseconds unless you call Stop() method.

If Enabled is set to true and AutoReset is set to false, the Timer raises the Elapsed event only once, the first time the interval elapses. When Enabled is true and AutoReset is true, the Timer continues to raise the Elapsed event on the specified interval.

from Timer.Start Method

Default for AutoReset is true, so the second sentence describes your case.

You current code starts new Timer every 50 seconds without stopping or removing the old one. That's why you get that high CPU and memory usage.

Change daily_Elapsed to:

private void daily_Elapsed(object sender, EventArgs e)
{
    string time_rightmeow = DateTime.Now.ToString("hh:mm tt");
    if (time_rightmeow == Configuration.Day_time)
    {
        day_backup();
    }

}

Nice solution but why doesn't the garbage collector dispose of all the redundant `Timer` objects? Indeed, why doesn't the GC dispose of _the first_ `Timer` object before its interval has elapsed? After all the only reference to the object is local variable `timer2` in `day_timer()`, and when the method ends the GC should be free to dispose of the object. But the object is still around 50 sec. later, due to the event handler delegate created in `day_timer()`. — groverboy, Nov 19 '13 at 03:13
For those not familiar with this gotcha, see [this question](http://stackoverflow.com/questions/4526829/why-and-how-to-avoid-event-handler-memory-leaks). — groverboy, Nov 19 '13 at 03:16

How to fix a memory issue with my timer loop

1 Answers1