Reputation: 5241
I have a Windows service that uses the producer/consumer queue model with multiple worker threads processing tasks off a queue. These tasks can be very long running, in the order of many minutes if not hours, and do not involve loops.
My question is about the best way to handle the service stop to gracefully end processing on these worker threads. I have read in another SO question that using thread.Abort() is a sign of bad design, but it seems that the service OnStop() method is only given a limited amount of time to finish before the service is terminated. I can do sufficient clean-up in the catch for ThreadAbortException (there is no danger of inconsistent state) so calling thread.Abort() on the worker threads seems OK to me. Is it? What are the alternatives?
Upvotes: 10
Views: 10102
Reputation: 18765
The question as amended actually has less to do with threading and more to do with how to stop long running actions. Personally I always use APM for lengthy stream and communications activities such as large file transfers. Each callback runs on an IO completion pool thread and completes quickly, processing a modest chunk and scheduling the next pass. Pending operations can be cancelled simply by calling Close()
on the socket object. This is a great deal cheaper and more efficient than DIY thread management.
As already mentioned, Abort()
is bad karma and should be avoided.
The material below was written prior to exclusion of the looping case from the question.
When long running processes loop they should all include an exit flag in their loop condition so that you can signal them to exit.
bool _run; //member of service class
//in OnStart
_run = true;
//in a method on some other thread
while ((yourLoopCondition) & _run)
{
//do stuff
foreach (thing in things)
{
//do more stuff
if (!_run) break;
}
}
if (!_run) CleanUp();
//in OnStop
_run = false;
Strictly you ought to use volatiles but since only the control logic ever sets the flag it doesn't matter. Technically a race condition exists but that just means you might go round one more time.
Upvotes: 1
Reputation: 6554
With .NET 4.0 you can utilize the System.Threading.Tasks
namespace to take advantage of the Task
object. In a nutshell, you can assign a CancellationToken
to more gracefully handle cancellations/aborts in tasks, be it long or short running.
See here for more details from MSDN.
Upvotes: 2
Reputation: 46044
If your process is being shutdown anyway, I personally don't see the problem with using Abort(). I would try to find another way, but in the end, it doesn't matter if you'll be doing clean-up in the main thread anyway.
Another option would be to mark your worker threads as background threads. That way, they'll close automatically when the process closes. You might be able to use the AppDomain.ProcessExit event to clean up before exiting.
Upvotes: 0
Reputation: 19004
Here's a code I use for stopping threads in a Windows service (mind you I use Threads directly and not using thread pools):
// signal all threads to stop
this.AllThreadsStopSignal.Set();
if (logThreads.IsDebugEnabled)
logThreads.Debug ("Stop workers");
// remember the time of the signal
DateTime signalTime = DateTime.Now;
// create an array of workers to be stopped
List<IServiceWorker> workersToBeStopped = new List<IServiceWorker> (workers);
while (true)
{
// wait for some time
Thread.Sleep (1000);
// go through the list and see if any workers have stopped
int i = 0;
while (i < workersToBeStopped.Count)
{
IServiceWorker workerToBeStopped = workersToBeStopped [i];
if (log.IsDebugEnabled)
log.Debug (String.Format (System.Globalization.CultureInfo.InvariantCulture,
"Stopping worker '{0}'. Worker state={1}",
workerToBeStopped.WorkerDescription,
workerToBeStopped.WorkerState));
bool stopped = workerToBeStopped.JoinThread (TimeSpan.Zero);
// if stopped, remove it from the list
if (stopped)
{
workersToBeStopped.RemoveAt (i);
if (log.IsDebugEnabled)
log.Debug (String.Format (System.Globalization.CultureInfo.InvariantCulture,
"Worker '{0}' was stopped.", workerToBeStopped.WorkerDescription));
}
else
{
i++;
if (log.IsDebugEnabled)
log.Debug (String.Format (System.Globalization.CultureInfo.InvariantCulture,
"Worker '{0}' could not be stopped, will try again later. Worker state={1}",
workerToBeStopped.WorkerDescription,
workerToBeStopped.WorkerState));
}
}
// if all workers were stopped, exit from the loop
if (workersToBeStopped.Count == 0)
break;
// check if the duration of stopping has exceeded maximum time
DateTime nowTime = DateTime.Now;
TimeSpan duration = nowTime - signalTime;
if (duration > serviceCustomization.ThreadTerminationMaxDuration)
{
// execute forced abortion of all workers which have not stopped
foreach (IServiceWorker worker in workersToBeStopped)
{
try
{
log.Warn (String.Format (System.Globalization.CultureInfo.InvariantCulture,
"Aborting worker '{0}.", worker.WorkerDescription));
worker.Abort ();
log.Warn (String.Format (System.Globalization.CultureInfo.InvariantCulture,
"Worker '{0}' aborted.", worker.WorkerDescription));
}
catch (ThreadStateException ex)
{
log.Warn (String.Format (System.Globalization.CultureInfo.InvariantCulture,
"Worker '{0}' could not be aborted.", worker.WorkerDescription), ex);
}
}
break;
}
}
Upvotes: 0
Reputation: 8174
Use ManualResetEvent to check if event is signalized, see sample at Thread Synchronization (C# Programming Guide)
Upvotes: 0
Reputation: 109005
Create a task type for "shutdown" and inject that into your producer/consumer queue once for each worker thread.
Then use Thread.Join (with a timeout) to ensure shutdown has completed.
Upvotes: 5
Reputation: 1062780
Indeed, Abort
should be avoided. It would be best to give them some time to exit gracefully - then perhaps after a timeout, maybe consider aborting them - but ultimately, service stop can do that just the same by killing the process instead.
I would try to have a signal in my queues that says "flush and exit" - much like the Close
method here, but with some kind of signal when complete.
If you resort to Abort
- consider the process fatally wounded. Kill it ASAP.
Upvotes: 6