29

I'm writing a Windows service that runs a variable length activity at intervals (a database scan and update). I need this task to run frequently, but the code to handle isn't safe to run multiple times concurrently.

How can I most simply set up a timer to run the task every 30 seconds while never overlapping executions? (I'm assuming System.Threading.Timer is the correct timer for this job, but could be mistaken).

bluish
  • 26,356
  • 27
  • 122
  • 180
JoshRivers
  • 9,920
  • 8
  • 39
  • 39
  • I don't know if you have solved this yet but Monitor.Tryenter with a lock is the way to go. After the interval code is only executed if the lock can be obtained. You only get the lock if the thread is not already executing your code. – Herman Van Der Blom Apr 17 '19 at 18:01

7 Answers7

40

You could do it with a Timer, but you would need to have some form of locking on your database scan and update. A simple lock to synchronize may be enough to prevent multiple runs from occurring.

That being said, it might be better to start a timer AFTER your operation is complete, and just use it one time, then stop it. Restart it after your next operation. This would give you 30 seconds (or N seconds) between events, with no chance of overlaps, and no locking.

Example :

System.Threading.Timer timer = null;

timer = new System.Threading.Timer((g) =>
  {
      Console.WriteLine(1); //do whatever

      timer.Change(5000, Timeout.Infinite);
  }, null, 0, Timeout.Infinite);

Work immediately .....Finish...wait 5 sec....Work immediately .....Finish...wait 5 sec....

InteXX
  • 6,135
  • 6
  • 43
  • 80
Reed Copsey
  • 554,122
  • 78
  • 1,158
  • 1,373
  • 3
    I second this approach - 'it might be better to start a timer AFTER you're operation is complete...' – hitec Mar 26 '09 at 05:26
  • 4
    I third this approach. You could also calculate the timer delay dynamically to get closer to the 30 seconds. Disable the timer, get the system time, update the database, then initialize the next timer to fire 30 seconds after the saved time. With a minimum timer delay for safety. – mghie Mar 26 '09 at 06:18
  • how we can be sure that operation will be complete in 5 secs. upvote totals more than @jsw, but its look like more effective. – Nuri YILMAZ Oct 21 '17 at 11:21
  • 1
    Why is it so hard to find this solution.. took me hours – gameon67 Aug 23 '19 at 09:59
  • if i refresh the page it overlaps ! – mohamed elyamani Nov 19 '19 at 10:14
29

I'd use Monitor.TryEnter in your elapsed code:

if (Monitor.TryEnter(lockobj))
{
  try
  {
    // we got the lock, do your work
  }
  finally
  {
     Monitor.Exit(lockobj);
  }
}
else
{
  // another elapsed has the lock
}
jsw
  • 1,752
  • 1
  • 14
  • 20
19

I prefer System.Threading.Timer for things like this, because I don't have to go through the event handling mechanism:

Timer UpdateTimer = new Timer(UpdateCallback, null, 30000, 30000);

object updateLock = new object();
void UpdateCallback(object state)
{
    if (Monitor.TryEnter(updateLock))
    {
        try
        {
            // do stuff here
        }
        finally
        {
            Monitor.Exit(updateLock);
        }
    }
    else
    {
        // previous timer tick took too long.
        // so do nothing this time through.
    }
}

You can eliminate the need for the lock by making the timer a one-shot and re-starting it after every update:

// Initialize timer as a one-shot
Timer UpdateTimer = new Timer(UpdateCallback, null, 30000, Timeout.Infinite);

void UpdateCallback(object state)
{
    // do stuff here
    // re-enable the timer
    UpdateTimer.Change(30000, Timeout.Infinite);
}
Jim Mischel
  • 131,090
  • 20
  • 188
  • 351
  • @RollerCosta Please explain why you think that Option 2 creates multiple threads. Everything I've read, and my experience, tells me it doesn't. The timer is a one-shot, meaning that it fires once. Then in the callback I re-enable it, again as a one-shot. – Jim Mischel Oct 02 '18 at 03:35
  • Thats what we had in our Windows Service but that didnt work. The overlap is not handled like it should. – Herman Van Der Blom Apr 16 '19 at 10:19
  • @HermanVanDerBlom I presented two different solutions. Which one did you have in your service, and how did it not work? – Jim Mischel Apr 16 '19 at 13:27
  • I use the locking. That works. Start and stop not because if it's put in a windows service and the service is stopped, the timer could start again while already disposed. I derived a Timer class with a Is Disposed method and a new Start method that checked if the Timer is already disposed. After this solution I used the lock and found that a better solution. The case is not that it blocks but that there can be no overlap now, so start/stop is not necessary and feels wrong – Herman Van Der Blom Apr 17 '19 at 17:55
3

Starting from .NET 6 there is a new timer available, the PeriodicTimer. This is a lightweight async-enabled timer, that becomes the perfect tool when overlapping executions should be strictly forbidden. You use this timer by writing an asynchronous method with a loop, and invoking it to start the loop:

private Task _operation;
private CancellationTokenSource _operationCancellation = new();

//...
_operation = StartTimer();
//...

private async Task StartTimer()
{
    PeriodicTimer timer = new(TimeSpan.FromSeconds(30));
    while (true)
    {
        await timer.WaitForNextTickAsync(_operationCancellation.Token);
        try
        {
            DoSomething();
        }
        catch (Exception ex)
        {
            _logger.LogError(ex);
        }
    }
}

Instead of using a CancellationTokenSource, you can also stop the loop by disposing the PeriodicTimer. In this case the await timer.WaitForNextTickAsync() will return false.

It is possible that the DoSomething will be invoked subsequently with smaller interval than 30 seconds, but it's impossible that it will be invoked in overlapping fashion, unless you start accidentally two asynchronous loops.

This timer does not support disabling and reenabling it. If you need this functionality you could look at the third-party Nito.AsyncEx.PauseTokenSource component.

In case you are targeting a .NET version earlier than .NET 6, you could look at this question for an alternative: Run async method regularly with specified interval.

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
2

instead of locking (which could cause all of your timed scans to wait and eventually stack up). You could start the scan/update in a thread and then just do a check to see if the thread is still alive.

Thread updateDBThread = new Thread(MyUpdateMethod);

...

private void timer_Elapsed(object sender, ElapsedEventArgs e)
{
    if(!updateDBThread.IsAlive)
        updateDBThread.Start();
}
Steven Evers
  • 16,649
  • 19
  • 79
  • 126
  • True, but if you ran the scan/update within the elapsed, you couldn't get a handle on it to check to see if it was alive. – Steven Evers Mar 26 '09 at 15:00
1

I've used a mutex when I've wanted single execution:

    private void OnMsgTimer(object sender, ElapsedEventArgs args)
    {
        // mutex creates a single instance in this application
        bool wasMutexCreatedNew = false;
        using(Mutex onlyOne = new Mutex(true, GetMutexName(), out wasMutexCreatedNew))
        {
            if (wasMutexCreatedNew)
            {
                try
                {
                      //<your code here>
                }
                finally
                {
                    onlyOne.ReleaseMutex();
                }
            }
        }

    }

Sorry I'm so late...You will need to provide the mutex name as part of the GetMutexName() method call.

sscheider
  • 522
  • 5
  • 14
1

You could use the AutoResetEvent as follows:

// Somewhere else in the code
using System;
using System.Threading;

// In the class or whever appropriate
static AutoResetEvent autoEvent = new AutoResetEvent(false);

void MyWorkerThread()
{
   while(1)
   {
     // Wait for work method to signal.
        if(autoEvent.WaitOne(30000, false))
        {
            // Signalled time to quit
            return;
        }
        else
        {
            // grab a lock
            // do the work
            // Whatever...
        }
   }
}

A slightly "smarter" solution is as follow in pseudo-code:

using System;
using System.Diagnostics;
using System.Threading;

// In the class or whever appropriate
static AutoResetEvent autoEvent = new AutoResetEvent(false);

void MyWorkerThread()
{
  Stopwatch stopWatch = new Stopwatch();
  TimeSpan Second30 = new TimeSpan(0,0,30);
  TimeSpan SecondsZero = new TimeSpan(0);
  TimeSpan waitTime = Second30 - SecondsZero;
  TimeSpan interval;

  while(1)
  {
    // Wait for work method to signal.
    if(autoEvent.WaitOne(waitTime, false))
    {
        // Signalled time to quit
        return;
    }
    else
    {
        stopWatch.Start();
        // grab a lock
        // do the work
        // Whatever...
        stopwatch.stop();
        interval = stopwatch.Elapsed;
        if (interval < Seconds30)
        {
           waitTime = Seconds30 - interval;
        }
        else
        {
           waitTime = SecondsZero;
        }
     }
   }
 }

Either of these has the advantage that you can shutdown the thread, just by signaling the event.


Edit

I should add, that this code makes the assumption that you only have one of these MyWorkerThreads() running, otherwise they would run concurrently.

grieve
  • 13,220
  • 10
  • 49
  • 61