Thread Sleep and Windows Services

Question

I'm working on a Windows Service that's having some issues with Thread.Sleep() so I figured I would try to use a timer instead as this question recommends:

Using Thread.Sleep() in a Windows Service

Thing is it's not entirely clear to me how one might implement this. I believe this is the way but I just wanted to make sure:

'' Inside The Service Object
Dim closingGate As System.Threading.AutoResetEvent

Protected Overrides Sub OnStart(ByVal args() As String)
   Dim worker As New Threading.Thread(AddressOf Work)
   worker.Start()
End Sub

Protected Sub Work()
   Dim Program = New MyProgram()
   closingGate = New System.Threading.AutoResetEvent(False)
   Program.RunService(closingGate)
End Sub

Protected Overrides Sub OnStop()
   closingGate.Set()
End Sub

'' Inside My Programs Code:
Public Sub RunService(closingGate As AutoResetEvent)
   Dim tmr As New Timer
   '' ...and so on

   closingGate.WaitOne()
End Sub

Aside from using VB.Net (Kidding, but I'd be using C# if I could.) am I on the right track here? Is this better than using Thread.Sleep()?

Answer 1

Honestly, I would have to say that I think you're a little off the beaten path here.

First of all, the actual code posted doesn't really make sense; the worker thread is waiting for a signal but for no reason - it's not actually in a loop of any kind or waiting for a resource. Second, if you did need to execute some (perhaps omitted) cleanup code in the worker after receiving the shutdown signal, your service might not give the worker thread enough time to clean up.

But, more fundamentally, all you've done is move the problem to a separate thread. This may keep the service responsive to the service controller but it doesn't fix the design problem - and it adds a lot of unnecessary complexity by using the threads and mutexes; ultimately it's just going to make your service harder to debug if you ever need to.

Let's say you need to execute some code every 5 seconds. The idea is, instead of "waiting" 5 seconds, you invert the control, and let the timer invoke your work.

This is bad:

protected override void OnStart(string[] args)
{
    while (true)
    {
        RunScheduledTasks();    // This is your "work"
        Thread.Sleep(5000);
    }
}

This is good:

public class MyService : ServiceBase
{
    private Timer workTimer;    // System.Threading.Timer

    protected override void OnStart(string[] args)
    {
        workTimer = new Timer(new TimerCallback(DoWork), null, 5000, 5000);
        base.OnStart(args);
    }

    protected override void OnStop()
    {
        workTimer.Dispose();
        base.OnStop();
    }

    private void DoWork(object state)
    {
        RunScheduledTasks();  // Do some work
    }
}

That's it. That's all you have to do to do work at regular intervals. As long as the work itself doesn't run for seconds at a time, your service will remain responsive to the service controller. You can even support pausing under this scenario:

protected override void OnPause()
{
    workTimer.Change(Timeout.Infinite, Timeout.Infinite);
    base.OnPause();
}

protected override void OnContinue()
{
    workTimer.Change(0, 5000);
    base.OnContinue();
}

The only time when you need to start creating worker threads in your service is when the work itself can run for a very long time and you need the ability to cancel in the middle. That tends to involve a good deal of design effort, so I won't get into that without knowing for sure that it's related to your scenario.

It's best not to start introducing multithreaded semantics into your service unless you really need it. If you're just trying to schedule small-ish units of work to be done, you definitely don't need it.

Answer 2

I recently wrote a service that had to do some work, then wait n minutes, then loop.

I also used a ManualResetEvent (called _evt), and a call to Set() in OnStop. But I didn't need a timer.

In the service thread, I had:

for (;;) {

   // do some work
   if (_evt.WaitOne(_waitInterval)) {

       // got the signal => time to leave
       break;
   }
}

So, either a timeout occurred => time for some more work. Or Set() is called and the loop is terminated.