48

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
   }
}
Doug Porter
  • 7,721
  • 4
  • 40
  • 55
Adonis L
  • 1,679
  • 4
  • 20
  • 23

11 Answers11

43

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
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • 16
    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 Jul 07 '09 at 11:21
35
class Program
{
    static void Main(string[] args)
    {
        Timer timer = new Timer(new TimerCallback(TimeCallBack),null,1000,50000);
        Console.Read();
        timer.Dispose();
    }

    public static void TimeCallBack(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

Scott Stafford
  • 43,764
  • 28
  • 129
  • 177
Prashanth
  • 2,404
  • 1
  • 17
  • 19
  • 6
    Note that the timer might be subject to additional system stalls or high loads that the sleep might not. This should be considered in anyone's implementation. – dyasta May 19 '12 at 21:00
11

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.

7

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
  • 168,566
  • 31
  • 210
  • 223
4

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
  • 2,969
  • 21
  • 20
2

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(condition && !error_condition)

Also, I'd go with a Timer.

Lakhwinder Singh
  • 5,536
  • 5
  • 27
  • 52
cjk
  • 45,739
  • 9
  • 81
  • 112
1

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.

Community
  • 1
  • 1
Simon Temlett
  • 2,397
  • 2
  • 17
  • 17
1

I have used both timers and Thread.Sleep(x), or either, depending on the situation.

If I have a short piece of code that needs to run repeadedly, I probably use a timer.

If I have a piece of code that might take longer to run than the delay timer (such as retrieving files from a remote server via FTP, where I don't control or know the network delay or file sizes / count), I will wait for a fixed period of time between cycles.

Both are valid, but as pointed out earlier they do different things. The timer runs your code every x milliseconds, even if the previous instance hasn't finished. The Thread.Sleep(x) waits for a period of time after completing each iteration, so the total delay per loop will always be longer (perhaps not by much) than the sleep period.

jwhitley
  • 11
  • 1
0

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

Cédric Bignon
  • 12,892
  • 3
  • 39
  • 51
Umair Ahmed
  • 11,238
  • 5
  • 33
  • 39
0

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.

Nick DeMayo
  • 1,086
  • 2
  • 15
  • 23
-1

I would have to say a sleep is a better implementation with a state machine behind it. This would still keep you in control of the application at all times, but allowing any response needed at any specific time. This also will handle timer callbacks that are shorter than the "Processing execution time in the loop"

For example..

<!-- language: c# -->
public enum State
{
    Idle = 0,
    Processing = 1,
    Stop = 100,
}
public void Run()
{
    State state = State.Idle;   // could be a member variable, so a service could stop this too

    double intervalInSeconds = 60;
    System.DateTime nextExecution = System.DateTime.Now.AddSeconds(intervalInSeconds);
    while (state != State.Stop)
    {
        switch (state)
        {
            case State.Idle:
                {
                    if (nextExecution > System.DateTime.Now)
                    {
                        state = State.Processing;
                    }
                }
                break;
            case State.Processing:
                {
                    // do your once-per-minute code here

                    // if you want it to stop, just set it state to stop.
                    // if this was a service, you could stop execution by setting state to stop, also
                    // only time it would not stop is if it was waiting for the process to finish, which you can handle in other ways

                    state = State.Idle;
                    nextExecution = System.DateTime.Now.AddSeconds(intervalInSeconds);
                }
                break;
            default:
                break;
        }

        System.Threading.Thread.Sleep(1);
    }
}
Cr4t3r
  • 1
  • 1
    And what advantage do you feel sleeping adds over using a timer. A number of others have given disadvantages of sleeping which you have not addressed. – Servy Jan 30 '13 at 17:08
  • my angle is to maintain control in the app at all times, a sleep and/or a long timer will not allow you to process things at intervals smaller than the selected wait period. – Cr4t3r Jan 30 '13 at 17:12
  • 2
    That's a problem with a naive `Sleep` implementation, which you improve on. It's not even a problem in a `Timer` based solution; you don't define a wait interval, you define an interval of when to fire the event, so none of this business of determining how long to wait needs to be done at all. – Servy Jan 30 '13 at 17:15