Alan Coromano
Alan Coromano

Reputation: 26008

System.Threading.Timer in C# it seems to be not working. It runs very fast every 3 second

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?

Upvotes: 118

Views: 195467

Answers (5)

Marco Mp
Marco Mp

Reputation: 422

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).

Upvotes: 9

Umka
Umka

Reputation: 1

 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.

Upvotes: 0

Ivan Zlatanov
Ivan Zlatanov

Reputation: 5226

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.

Upvotes: 253

Ivan Leonenko
Ivan Leonenko

Reputation: 2383

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);
     }
   }
 }

Upvotes: 14

Damien_The_Unbeliever
Damien_The_Unbeliever

Reputation: 239664

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.

Upvotes: 3

Related Questions