Stopping timer in its callback method

Question

I have a System.Threading.Timer that calls its appropriate event handler (callback) every 10 ms. The method itself is not reentrant and can sometimes take way longer than 10 ms. Thus, I want to stop the timer during method execution.

Code:

private Timer _creatorTimer;

// BackgroundWorker's work
private void CreatorWork(object sender, DoWorkEventArgs e) {
      _creatorTimer = new Timer(CreatorLoop, null, 0, 10);

      // some other code that worker is doing while the timer is active
      // ...
      // ...
}

private void CreatorLoop(object state) {
      // Stop timer (prevent reentering)
      _creatorTimer.Change(Timeout.Infinite, 0);

      /*
          ... Work here
      */

      // Reenable timer
      _creatorTimer.Change(10, 0);
} 

MSDN states that the callback method is called (every time the timer fires) in separate thread from the thread pool. That means that if I stop the timer the first thing in method it still doesn't neccessarily prevent the timer to fire and run another instance of the method before the first one had a chance to stop the timer.

Should maybe the timer (or even the non-reentrant method itself) be locked? What is the right way to prevent timer from firing during execution of its callback (and non-reentrant) method?

Jevons

This question may help you out #1116749 – Windsor

Answer 1

You could let the timer continue firing the callback method but wrap your non-reentrant code in a Monitor.TryEnter/Exit. No need to stop/restart the timer in that case; overlapping calls will not acquire the lock and return immediately.

 private void CreatorLoop(object state) 
 {
   if (Monitor.TryEnter(lockObject))
   {
     try
     {
       // Work here
     }
     finally
     {
       Monitor.Exit(lockObject);
     }
   }
 }

Diagnosis

+1 I never really thought of a use for TryEnter; that is very interesting. – Hayman

This seems to be doing the trick. Two or more threads may enter the method, but only one will actually work. I have also stopped the timer after Monitor.TryEnter() so it does not fire at all during execution (there is no need for it to fire), just in case the execution time is much greater than the timer's period. Timer is restarted after the work in method completes. – Jevons

Depends on the application owner requirement. In you solution timer will be called every 10 sec so if will not enter next try will be for 20 seconds. When we stop/start timer then next invoke of the Handler will be 10 secondes after finishing handler before. – Thorr

Answer 2

A couple possible solutions:

• have the real work done in yet another thread delegate that's waiting on an event. The timer callback merely signals the event. The worker thread cannot be reentered, as it's a single thread that does its work only when the event is signaled. The timer is reentrant, since all it does is signal the event (seems a little roundabout and wasteful, but it'll work)
• have the timer created with only a start timeout and no periodic timeout so it'll fire only once. The timer callback will dispose of that timer object and create a new one when it has completed its work that will also only fire once.

You may be able to manage option #2 without disposing/creating a new object by using the Change() method of the original timer object, but I'm not sure what the behavior is exactly of calling Change() with a new start timeout after the first timeout has expired. That would be worth a test or two.

Edit:

---

I did the test - manipulating the timer as a restartable one-shot seems to work perfectly, and it's much simpler than the other methods. Here's some sample code based on yours as a starting point (a few details may have changed to get it to compile on my machine):

private Timer _creatorTimer;

// BackgroundWorker's work
private void CreatorWork(object sender, EventArgs e) {
    // note: there's only a start timeout, and no repeat timeout
    //   so this will fire only once
    _creatorTimer = new Timer(CreatorLoop, null, 1000, Timeout.Infinite);

    // some other code that worker is doing while the timer is active
    // ...
    // ...
}

private void CreatorLoop(object state) {
    Console.WriteLine( "In CreatorLoop...");
    /*
        ... Work here
    */
    Thread.Sleep( 3000);

    // Reenable timer
    Console.WriteLine( "Exiting...");

    // now we reset the timer's start time, so it'll fire again
    //   there's no chance of reentrancy, except for actually
    //   exiting the method (and there's no danger even if that
    //   happens because it's safe at this point).
    _creatorTimer.Change(1000, Timeout.Infinite);
}

Redden

This seems to work, yes, but the code becomes less clean when there's many outcomes of the method (many if-else branches and exceptions). But seems to be a good solution when it comes to performance because no sync mechanisms are used. Other than that, this WILL NOT work if there are other methods/threads/timers that can try to enter this method. Then of course, we are reentering non-reentrant method, which is where monitors work better. Thanks for solution and the test, anyway. It's a nice idea. – Jevons

The complexity of the code is no more of an issue than if you're using a mutex - just wrap the code in a try/finally or simply call another routine that has the complexity and leave the timer callback routine a simple 'call then reset the timer'. If the callback will be used by multiple timers, then yes this technique won't work and you'll need a true synchronization object. I find that it's pretty uncommon for a timer callback to be used by multiple timers - particularly when the callback is very complex (that generally means the timer callback is designed for a very specific purpose). – Redden

You can achieve this same behaviour using System.Timers.Timer which I think is much easier. See https://mcmap.net/q/112938/-non-reentrant-timers – Nebraska

Answer 3

I've had similar situation with a System.Timers.Timer, where the elapsed event is executed from a threadpool and needs to be reentrant.

I used this method to get around the issue:

private void tmr_Elapsed(object sender, EventArgs e)
{
    tmr.Enabled = false;
    // Do Stuff
    tmr.Enabled = true;
}

Depending on what you're doing you may want to consider a System.Timers.Timer, here's a nice summary from MSDN

                                         System.Windows.Forms    System.Timers         System.Threading  
Timer event runs on what thread?         UI thread               UI or worker thread   Worker thread
Instances are thread safe?               No                      Yes                   No
Familiar/intuitive object model?         Yes                     Yes                   No
Requires Windows Forms?                  Yes                     No                    No
Metronome-quality beat?                  No                      Yes*                  Yes*
Timer event supports state object?       No                      No                    Yes
Initial timer event can be scheduled?    No                      No                    Yes
Class supports inheritance?              Yes                     Yes                   No

* Depending on the availability of system resources (for example, worker threads)            

Wunderlich

I believe that this does not actually get around the issue. Well, it may in 99.9% cases, but if the system does not give processor time to your event handler before the next timer's Elapsed event firing, two different threads may execute the method in parallel. – Jevons

Good point! You could always use this in conjunction with a locking solution like jsw's – Wunderlich

Answer 4

I do it with Interlocked that provides atomic operations, and by CompareExchange ensures that only one thread at a time enters the critical section:

private int syncPoint = 0;

private void Loop()
    {
        int sync = Interlocked.CompareExchange(ref syncPoint, 1, 0);
         //ensures that only one timer set the syncPoint to  1 from 0
        if (sync == 0)
        {
            try
            {
               ...
            }
            catch (Exception pE)
            {
               ...  
            }
            syncPoint = 0;
        }

    }

Crabber

Answer 5

    //using Timer with callback on System.Threading namespace
    //  Timer(TimerCallback callback, object state, int dueTime, int period);
    //      TimerCallback: delegate to callback on timer lapse
    //      state: an object containig information for the callback
    //      dueTime: time delay before callback is invoked; in milliseconds; 0 immediate
    //      period: interval between invocation of callback; System.Threading.Timeout.Infinity to disable
    // EXCEPTIONS:
    //      ArgumentOutOfRangeException: negative duration or period
    //      ArgumentNullException: callback parameter is null 

    public class Program
    {
        public void Main()
        {
            var te = new TimerExample(1000, 2000, 2);
        }
    }

    public class TimerExample
    {
        public TimerExample(int delayTime, int intervalTime, int treshold)
        {
            this.DelayTime = delayTime;
            this.IntervalTime = intervalTime;
            this.Treshold = treshold;
            this.Timer = new Timer(this.TimerCallbackWorker, new StateInfo(), delayTime, intervalTime);
        }

        public int DelayTime
        {
            get;
            set;
        }

        public int IntervalTime
        {
            get;
            set;
        }

        public Timer Timer
        {
            get;
            set;
        }

        public StateInfo SI
        {
            get;
            set;
        }

        public int Treshold
        {
            get;
            private set;
        }

        public void TimerCallbackWorker(object state)
        {
            var si = state as StateInfo;

            if (si == null)
            {
                throw new ArgumentNullException("state");
            }

            si.ExecutionCounter++;

            if (si.ExecutionCounter > this.Treshold)
            {
                this.Timer.Change(Timeout.Infinite, Timeout.Infinite);
                Console.WriteLine("-Timer stop, execution reached treshold {0}", this.Treshold);
            }
            else
            {
                Console.WriteLine("{0} lapse, Time {1}", si.ExecutionCounter, si.ToString());
            }
        }

        public class StateInfo
        {
            public int ExecutionCounter
            {
                get;
                set;
            }

            public DateTime LastRun
            {
                get
                {
                    return DateTime.Now;
                }
            }

            public override string ToString()
            {
                return this.LastRun.ToString();
            }
        }
    }

    // Result:
    // 
    //  1 lapse, Time 2015-02-13 01:28:39 AM
    //  2 lapse, Time 2015-02-13 01:28:41 AM
    //  -Timer stop, execution reached treshold 2
    // 

Cumulostratus

I'd suggest using a code formatter or something to tidy up that code, re. spacing, formatting etc. It may be wonderful, but first impressions count, and my first impression if that I would not want to use code looking like that. – Marji

It was a quick but working example, hope it is better readable now. – Cumulostratus