views:

103

answers:

6

I need to inform a so-called worker thread to stop work at the next available oppertunity. Currently I'm using something like this:

    public void Run()
    {
        while (!StopRequested)
            DoWork();
    }

The concern I have is that StopRequested is being set to True on a different thread. Is this safe? I know I can't lock a boolean. Perhaps there's a different way to inform a thread that a stop is required. For example I would be happy to check Thread.CurrentThread.ThreadState == ThreadState.StopRequested but it's not clear how I can set the thread state as such.

+1  A: 

Using Event object will be better probably. Especially if your thread blocks. (Then you can still wait for the event inside the blocking call)

EFraim
+2  A: 

This is a better approach than trying to set the ThreadState. If you look at the docs for ThreadState, it specifically says that StopRequested is for internal use only.

Setting a boolean from another thread is a safe operation. You shouldn't need to lock in this case.

Reed Copsey
+2  A: 

You may want to look at using a BackgroundWorker to do the work for you, using RunWorkerAsync() and handling the worker.CancellationPending == true case during your logic.

BackgroundWorker worker = new BackgroundWorker();
worker.WorkerSupportsCancellation = true;
worker.DoWork += MyWorkFunction;
worker.RunWorkerCompleted += MyWorkDoneSuccessfullyFunction;
worker.RunWorkerAsync();

Another solution would be to use a volatile bool to signal that you need to stop, which you could set anywhere within the class.

private volatile bool cancelWork = false;

public void CancelWork()
{
  cancelWork = true;
}
Will Eddins
e.Cancelled is only after the worker has been stopped, see my post below
SwDevMan81
@SwDevMan81: You're correct, editted that comment. A correct implementation for DoWork can be found here: http://msdn.microsoft.com/en-us/library/system.componentmodel.doworkeventargs.argument.aspx
Will Eddins
+1  A: 

You could use Thread.Abort() and wrap any necessary cleanup in the DoWork() method in a finally block.

Charlie
A: 

If you use the BackgroundWorker, you would call the CancelAsync function of the BackgroundWorker sets the CancellationPending Property to true, which is what you would look for in your loop:

public void Run(object sender, DoWorkEventArgs e)   
{      
   BackgroundWorker worker = sender as BackgroundWorker;
   if(worker != null)
   {
      while (!worker.CancellationPending)   
      {
         DoWork();  
      }
   }
}

The above would be the DoWork event code which is called when you run the RunWorkerAsync function.

SwDevMan81
A: 

The concern I have is that StopRequested is being set to True on a different thread. Is this safe?

You should make sure that the StopRequested field has the volatile keyword. See this other answer for the reason why.

Wim Coenen