tags:

views:

6964

answers:

9

Hello, I am running a windows service and using a loop and Thread.Sleep to repeat a task, would it be better to use a timer method?

If yes a code example would be great

I am currently using this code to repeat

    int curMinute;
int lastMinute = DateTime.Now.AddMinutes(-1).Minute;
while (condition)
{
   curMinute = DateTime.Now.Minute;
   if (lastMinute < curMinute) {
         // do your once-per-minute code here
         lastMinute = curMinute;
   }
   Thread.Sleep(50000);      // sleeps for 50 seconds
   if (error condition that would break you out of this) {
       break;      // leaves looping structure
   }
}
+4  A: 

It's important to understand that your code will sleep for 50 seconds between ending one loop, and starting the next...

A timer will call your loop every 50 seconds, which isn't exactly the same.

They're both valid, but a timer is probably what you're looking for here.

Christopher Karper
+1  A: 

Yes, using a Timer will free up a Thread that is currently spending most of its time sleeping. A Timer will also more accurately fire every minute so you probably won't need to keep track of lastMinute anymore.

Jon Norton
+17  A: 

A timer is a better idea, IMO. That way, if your service is asked to stop, it can respond to that very quickly, and just not call the timer tick handler again... if you're sleeping, the service manager will either have to wait 50 seconds or kill your thread, neither of which is terribly nice.

Jon Skeet
To avoid killing the thread, you can use a ManualResetEvent.WaitOne(50000) rather than a Thread.Sleep(). Then you can Set() the wait handle to signal the thread to terminate at it's earliest convenience.A timer would probably still be preferable in this case though.
Thorarin
A: 

Not quite answering the question, but rather than having

   if (error condition that would break you out of this) {
       break;      // leaves looping structure
   }

You should probably have

while(!error condition)

Also, I'd go with a Timer.

ck
A: 

You can use either one. but i think Sleep() is easy,clear and shorter to implement

Umair Ahmed
+5  A: 

class Program {

    static void Main(string[] args)
    {
        Timer timer = new Timer(new TimerCallback(TimCallBack),null,1000,50000);
        Console.Read();
        timer.Dispose();
    }

    public static void TimCallBack(object o)
    {
      curMinute = DateTime.Now.Minute;
      if (lastMinute < curMinute) {
       // do your once-per-minute code here
       lastMinute = curMinute;
    }
}

The code could resemble something like the one above

Prashanth
+3  A: 

Beware that calling Sleep() will freeze the service, so if the service is requested to stop, it won't react for the duration of the Sleep() call.

Philippe Leybaert
A: 

I required a thread to fire once every minute (see question here) and I've now used a DispatchTimer based on the answers I received.

The answers provide some references which you might find useful.

Simon Temlett
A: 

I agree as well, using a timer is the best option. I have tried a solution similar to yours in the past and started having issues where the loop would misfire, and I would have to wait for another Thread.Sleep() before it would fire again. Also, it did cause all sorts of issues with stopping the service, I would get constant errors about how it wasn't responding and had to be closed.

@Prashanth's code should be exactly what you need.

sunmorgus