How do I safely stop a C# .NET thread running in a Windows service?

Question

I am maintaining some code which looks something like this. It's a Windows service which does some work every 30 minutes. The ActualWorkDoneHere method takes about 30 seconds to run, but if it is stopped while running it can leave things in a bad state. What is the best way to prevent that from happening? Should I replace the While(true) with a boolean which is set to false in the onstop method (removing the thread Abort call)? Is there some way to tell if a thread is sleeping?

namespace WorkService
{
    public partial class WorkService : ServiceBase
    {
        private Thread _workerThread = null;

        public WorkService()
        {
            InitializeComponent();
        }

        protected override void OnStart(string[] args)
        {
            _workerThread = new Thread(new ThreadStart(DoWork));
            _workerThread.Start();
        }

        protected override void OnStop()
        {
            _workerThread.Abort();
        }

        static void DoWork()
        {
            int sleepMinutes = 30;

            while (true)
            {
                 ActualWorkDoneHere();

                 System.Threading.Thread.Sleep(new TimeSpan(0, sleepMinutes, 0));
            }
        }
    }
}

Answer 1

When I have something like this, I usually use a ManualResetEvent. This is set in the Stop() call. Then I wait with a timeout:

for (;;)
{
    if (_stop.WaitOne(timeout))
        break;
    DoSomething();
}

Answer 2

Implementing it yourself is the only safe option. Even if you find a way to find out if a thread is sleeping, you will still have a race condition if you try to kill it (because it potentially starts processing after you check and before you kill it).

Instead of Thread.Sleep you could e.g. sleep 500ms and check if the abort flag is still false, sleep another 500ms etc. before 30mins passes, then do the job, etc. (this would be a pragmatic approach). If you want something more elegant, you could use a ManualResetEvent with a timeout to wait for the main thread signalling that its time to abort.

Answer 3

Try using an autoreset flag to handle stopping of the service. In that case you would not have to perform thread abort. Have added the sample code below

namespace WorkService
{
    public partial class WorkService : ServiceBase
    {
    AutoResetEvent serviceStopEvent = new AutoResetEvent( false);

        public WorkService()
        {
            InitializeComponent();
        }

        protected override void OnStart(string[] args)
        {
            Thread workerThread = new Thread(new ThreadStart(DoWork));
            workerThread.Start();
        }

        protected override void OnStop()
        {
           serviceStopEvent.Set();
        }

        static void DoWork()
        {
            int sleepMinutes = 30;
        WaitHandle[ ] handles = new WaitHandle[ ] { serviceStopEvent };

            while (WaitHandle.WaitAny( handles))
            {
                 ActualWorkDoneHere();

            }
        }

    }
}

Cheers, Bharath.

Answer 4

Here's one way to do it. Add the following variables to your class:

private readonly object syncObject = new object();
private bool stopping;
private bool stopped = true;

Then in OnStart, you do something like this (I have a helper method which does some logging in this example, and the "Run" method does the actual work).:

    public override void OnStart()
    {
        while (stopping)
        {
            Thread.Sleep(MSECS_SLEEP_FOR_STOP);
        }

        lock (syncObject)
        {
            // make sure task isn't already started
            if (!stopped)
            {
                Helper.WriteToLog(logger, Level.INFO,
                    string.Format("{0} {1}", TASK_NAME, "is already started."));
                return;
            }
            stopped = false;
        }

        // start task in new thread
        Thread thread = new Thread(Run);
        thread.Start();

        Helper.WriteToLog(logger, Level.INFO,
            string.Format("{0} {1}", TASK_NAME, "was started."));
    }

Your "Run" method, which does the work of the thread, would look like this (processInterval would be how long you want to wait between runs, you could set it in the constructor or just hardcode it):

    private void Run()
    {
        try
        {
            while (!stopping)
            {
                // do work here

                // wait for process interval
                DateTime waitStart = DateTime.Now;
                while (((DateTime.Now - waitStart).TotalMilliseconds < processInterval) && !stopping)
                {
                    // give processing time to other threads
                    Thread.Sleep(MSECS_SLEEP_FOR_CHECK);
                }
            }
            lock (syncObject)
            {
                stopped = true;
                stopping = false;
            }

            Helper.WriteToLog(logger, Level.INFO,
                string.Format("{0} {1}", TASK_NAME, "was stopped."));
        }
        catch (Exception e)
        {
            // log the exception, but ignore it (i.e. don't throw it)
            Helper.LogException(logger, MethodBase.GetCurrentMethod(), e);
        }
    }

Then in OnStop, you would do this:

    public override void OnStop()
    {
        lock (syncObject)
        {
            if (stopping || stopped)
            {
                Helper.WriteToLog(logger, Level.INFO,
                    string.Format("{0} {1}", TASK_NAME, "is already stopped."));
                return;
            }
            stopping = true;
        }
    }

Answer 5

You could use a lock object to prevent the thread being stopped while your work is actually happening...

    private static readonly object _syncRoot = new object();

    protected override void OnStop()
    {
        lock (_syncRoot) 
        {
            _workerThread.Abort();
        }
    }

    static void DoWork()
    {
        int sleepMinutes = 30;

        while (true)
        {
             lock (_syncRoot) 
             {
                 ActualWorkDoneHere();
             }

             System.Threading.Thread.Sleep(new TimeSpan(0, sleepMinutes, 0));
        }
    }

You should be careful though - if your ActualWorkDoneHere() function takes too long, windows will report the service as failing to stop.

Answer 6

Wow everyone makes this so complicated. Use a Timer:

On races: The original post had a race in OnStop which has been fixed. As far as I know putting the service into a stopped state will not abort threadpool threads which are used to service the timer. The condition of the timer firing and the service being stopped at the same time is irrelevant. ActualWorkDoneHere() will either run, or not run. Both are acceptable conditions.

namespace WorkService
{
 public partial class WorkService : ServiceBase
 {
  protected const int sleepMinutes = 30;
  protected System.Timers.Timer _interval;
  protected bool _running = false;

  public WorkService()
  {
   InitializeComponent();
   _interval = new System.Timers.Timer();
   _interval.Elapsed += new ElapsedEventHandler(OnTimedEvent);
   _interval.Interval = sleepMinutes * 60 * 1000;
   _running = false;
  }

  protected override void OnStart(string[] args)
  {
   _running = true;
   _interval.Enabled = true;
  }

  protected override void OnStop()
  {
   _interval.Enabled = false;
   _running = false;
  }

  private static void OnTimedEvent(object source, ElapsedEventArgs e)
  {
      if(_running)
          ActualWorkDoneHere();
  }
 }
}

Answer 7

My service listens on a network socket so what I did is create a joined pair of network sockets and used the select system call to listen on both. If the joined pair reported ready to read I knew to shutdown the service.

This trick can be used to trigger an arbitrary number of threads to shut down so long as none of them actually read from the connected pair.