115

I've a timer object. I want it to be run every minute. Specifically, it should run a OnCallBack method and gets inactive while a OnCallBack method is running. Once a OnCallBack method finishes, it (a OnCallBack) restarts a timer.

Here is what I have right now:

private static Timer timer;

private static void Main()
{
    timer = new Timer(_ => OnCallBack(), null, 0, 1000 * 10); //every 10 seconds
    Console.ReadLine();
}

private static void OnCallBack()
{
    timer.Change(Timeout.Infinite, Timeout.Infinite); //stops the timer
    Thread.Sleep(3000); //doing some long operation
    timer.Change(0, 1000 * 10);  //restarts the timer
}

However, it seems to be not working. It runs very fast every 3 second. Even when if raise a period (1000*10). It seems like it turns a blind eye to 1000 * 10

What did I do wrong?

Robert Harvey
  • 178,213
  • 47
  • 333
  • 501
Alan Coromano
  • 24,958
  • 53
  • 135
  • 205

5 Answers5

247

This is not the correct usage of the System.Threading.Timer. When you instantiate the Timer, you should almost always do the following:

_timer = new Timer( Callback, null, TIME_INTERVAL_IN_MILLISECONDS, Timeout.Infinite );

This will instruct the timer to tick only once when the interval has elapsed. Then in your Callback function you Change the timer once the work has completed, not before. Example:

private void Callback( Object state )
{
    // Long running operation
   _timer.Change( TIME_INTERVAL_IN_MILLISECONDS, Timeout.Infinite );
}

Thus there is no need for locking mechanisms because there is no concurrency. The timer will fire the next callback after the next interval has elapsed + the time of the long running operation.

If you need to run your timer at exactly N milliseconds, then I suggest you measure the time of the long running operation using Stopwatch and then call the Change method appropriately:

private void Callback( Object state )
{
   Stopwatch watch = new Stopwatch();

   watch.Start();
   // Long running operation

   _timer.Change( Math.Max( 0, TIME_INTERVAL_IN_MILLISECONDS - watch.ElapsedMilliseconds ), Timeout.Infinite );
}

I strongly encourage anyone doing .NET and is using the CLR who hasn't read Jeffrey Richter's book - CLR via C#, to read is as soon as possible. Timers and thread pools are explained in great details there.

Robert Harvey
  • 178,213
  • 47
  • 333
  • 501
Ivan Zlatanov
  • 5,146
  • 3
  • 29
  • 45
  • 6
    I don't agree with that `private void Callback( Object state ) { // Long running operation _timer.Change( TIME_INTERVAL_IN_MILLISECONDS, Timeout.Infinite ); }`. `Callback` might be called again before an operation is completed. – Alan Coromano Oct 09 '12 at 12:31
  • 2
    What I meant was that `Long running operation` might took much more time then `TIME_INTERVAL_IN_MILLISECONDS`. What would happen then? – Alan Coromano Oct 09 '12 at 12:37
  • 33
    Callback will not be called again, this is the point. This is why we pass Timeout.Infinite as a second parameter. This basically means don't tick again for the timer. Then reschedule to tick after we have completed the operation. – Ivan Zlatanov Oct 09 '12 at 13:20
  • Newbie to threading here - do you think this is possible to do with a `ThreadPool`, if you pass in the timer? I'm thinking of a scenario where a new thread is spawned to do a job at a certain interval - and then relegated to the thread pool when complete. –  Mar 28 '14 at 02:52
  • 2
    The System.Threading.Timer is a thread pool timer, that is executing it's callbacks on the thread pool, not a dedicated thread. After the timer completes the callback routine, the thread that executed the callback goes back in the pool. – Ivan Zlatanov Mar 28 '14 at 08:20
  • Also, it is safer to specify the interval using the TimeSpan class rather than hard coded numbers. – Fike Rehman Jun 10 '16 at 21:41
  • Can sb explain, why this is the correct usage and simply specifying dueTime and interval in the constructor is not? – karollo Jul 30 '18 at 06:46
  • @KarolŻurowski That depends on the use case. For the OP, the answer stated here is correct, but if you just want a simple repeating timer, you can just use the constructor to set it once and forget about it. – Tom Lint Dec 13 '18 at 09:27
14

It is not necessary to stop timer, see nice solution from this post:

"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);
     }
   }
 }
Community
  • 1
  • 1
Ivan Leonenko
  • 2,363
  • 2
  • 27
  • 37
  • it's not for my case. I need to stop timer exactly. – Alan Coromano Oct 09 '12 at 09:04
  • Are you trying to prevent entering callback more then one time? If no what are you trying to achieve? – Ivan Leonenko Oct 09 '12 at 09:11
  • 1. Prevent entering to callback more than one time. 2. Prevent executing too much times. – Alan Coromano Oct 09 '12 at 09:13
  • This exactly what it does. #2 is not much overhead as long as it returns right after if statement if object is locked, especially if you have such big interval. – Ivan Leonenko Oct 09 '12 at 09:20
  • 1
    This does not guarantee that the code is called no less than after the last execution (a new tick of the timer could be fired a microsecond after a previous tick released the lock). It depends if this is a strict requirement or not (not entirely clear from the problem's description). – Marco Mp Oct 09 '12 at 09:26
9

Is using System.Threading.Timer mandatory?

If not, System.Timers.Timer has handy Start() and Stop() methods (and an AutoReset property you can set to false, so that the Stop() is not needed and you simply call Start() after executing).

Yousha Aleayoub
  • 4,532
  • 4
  • 53
  • 64
Marco Mp
  • 422
  • 2
  • 6
  • 4
    Yes, but it could be a real requirement, or it just happened that timer was chosen because it's the one most used. Sadly .NET has tons of timer objects, overlapping for 90% but still being (sometimes subtly) different. Of course, if it is a requirement, this solution does not apply at all. – Marco Mp Oct 09 '12 at 12:44
  • 4
    As per the [documentation](https://msdn.microsoft.com/en-us/library/system.timers.timer(v=vs.110).aspx): The Systems.Timer class is available in the .NET Framework only. It is not included in the .NET Standard Library and is not available on other platforms, such as **.NET Core or the Universal Windows Platform. On these platforms, as well as for portability across all .NET platforms, you should use the System.Threading.Timer class instead.** – NotAgain May 11 '17 at 02:10
3

I would just do:

private static Timer timer;
 private static void Main()
 {
   timer = new Timer(_ => OnCallBack(), null, 1000 * 10,Timeout.Infinite); //in 10 seconds
   Console.ReadLine();
 }

  private static void OnCallBack()
  {
    timer.Dispose();
    Thread.Sleep(3000); //doing some long operation
    timer = new Timer(_ => OnCallBack(), null, 1000 * 10,Timeout.Infinite); //in 10 seconds
  }

And ignore the period parameter, since you're attempting to control the periodicy yourself.


Your original code is running as fast as possible, since you keep specifying 0 for the dueTime parameter. From Timer.Change:

If dueTime is zero (0), the callback method is invoked immediately.

Damien_The_Unbeliever
  • 234,701
  • 27
  • 340
  • 448
  • 2
    Is it necessary to dispose timer? Why don't you use `Change()` method? – Alan Coromano Oct 09 '12 at 09:02
  • 24
    Disposing the timer every time is absolutely unnecessary and wrong. – Ivan Zlatanov Oct 09 '12 at 09:57
  • @IvanZlatanov Can you please give additional insight on the same. When we call dispose, objects will be cleared from memory and this is good right? All I see is some additional process that might impact the solution. Just thinking loud... – Sudhakar Chavali Feb 05 '21 at 07:26
  • 1
    @SudhakarChavali You should always dispose your objects when you're done using them. In this example, because the timer has Change method which can enable / disable the instance, disposing it and creating new one is unnecessary. – Ivan Zlatanov Feb 05 '21 at 09:13
0
 var span = TimeSpan.FromMinutes(2);
 var t = Task.Factory.StartNew(async delegate / () =>
   {
        this.SomeAsync();
        await Task.Delay(span, source.Token);
  }, source.Token, TaskCreationOptions.LongRunning, TaskScheduler.Default);

source.Cancel(true/or not);

// or use ThreadPool(whit defaul options thread) like this
Task.Start(()=>{...}), source.Token)

if u like use some loop thread inside ...

public async void RunForestRun(CancellationToken token)
{
  var t = await Task.Factory.StartNew(async delegate
   {
       while (true)
       {
           await Task.Delay(TimeSpan.FromSeconds(1), token)
                 .ContinueWith(task => { Console.WriteLine("End delay"); });
           this.PrintConsole(1);
        }
    }, token) // drop thread options to default values;
}

// And somewhere there
source.Cancel();
//or
token.ThrowIfCancellationRequested(); // try/ catch block requred.
Umka
  • 1