How to start thread if button pressed and stop it if pressed again ?

Question

Hi all I'm using the next code to do what I'm asking for :

private delegate void CallerDelegate(object e);
CallerDelegate caler = new CallerDelegate(MethodToCall);

on button click event :

if (currBusyThrd != null && currBusyThrd.IsAlive)
   {
    currBusyThrd.Abort();
   }
ThreadPool.SetMaxThreads(1, 1);
//queue the work for thread processing
ThreadPool.QueueUserWorkItem(new WaitCallback(WaitCallbackMethod))

"WaitCallbackMethod" Method is :

void WaitCallbackMethod(object stateInfo)
  {
     // argList : i put some argument in a list to use it in "MethodToCall" ...
     BeginInvoke(caler,argList);
  }

and the method i'm calling by the thread is :

void MethodToCall(object args)
 {
 //Here I get the thread I'm calling to stop it when btn clicked again
 currBusyThrd = Thread.CurrentThread;

 // The rest of the code ...
 }

I feel that this is wrong ... How to do it right ?

Thanx in advance

Actually the calling will be by TextBox_KeyUp .. so every time the user enter a char the code will execute again .. and the BackgroundWorker didn't work .

Answer 1

One problem to this approach is that it's very dangerous to arbitrarily Abort a thread (in pretty much any language). There are too many issues that can popup around unfreed resources and misheld locks. It's typically best to set some kind of flag to ask the Thread to safely abort itself or to forget about the thread and let it run to completion.

Additionally, Aborting a Thread in the ThreadPool is very dangerous and I believe not a supported operation. The Threads in the ThreadPool are not owned by you and Aborting them cold have serious implications for the ThreadPool.

Here is the solution I would take.

private object m_lock = new object();
private bool m_isRunning = false;
private bool m_isAbortRequested = false;

public void OnButtonClick(object sender, EventArgs e) {
  lock ( m_lock ) {
    if ( m_isRunning ) {
      m_isAbortRequested = true;
    } else {
      m_isAbortRequested = false;
      m_isRunning = true;
      ThreadPool.QueueUserWorkItem(BackgroundMethod);
    }
  }
}

private void BackgroundMethod() {
  try {
    DoRealWork();
  } finally {
    lock (m_lock) {
      m_isRunning = false;
    }
  }
}

private void DoRealWork() {
  ...
  if ( m_isAbortRequested ) {
    return;
  }
}

Answer 2

Yes, this is very wrong. You should never try to manually control a ThreadPool thread. If you need this sort of control, you should be using your own Thread object. In addition, Abort() is not the recommended way of ending a thread; you should have a control volatile bool on your form that the code in MethodToCall checks at various points and exits gracefully when it's true. While you can use the same approach with the ThreadPool, the fact that you need to be able to cancel seems to indicate that the process is long-running, or at least has the potential to be. The ThreadPool shouldn't be used for long-running processes.

For example...

private volatile bool stopThread = false;
private Thread workThread;

private void StartThread()
{
    if(workThread == null)
    {
        stopThread = false;
        workThread = new Thread(new ThreadStart(MethodToCall));

        workThread.Start();
    }
}

private void StopThread()
{
    if(workThread != null)
    {
        stopThread = true;

        workThread.Join(); // This makes the code here pause until the Thread exits.

        workThread = null;
    }
}

Then in MethodToCall, just check the stopThread boolean at frequent intervals and do any cleanup work that you need to do and exit the method. For example...

private void MethodToCall()
{
    // do some work here and get to a logical stopping point

    if(stopThread)
    {
        // clean up your work

        return;
    }

    // do some more work and get to another stopping point

    if(stopThread)
    {
        // clean up your work

        return;
    }
}

And just repeat that pattern.

Answer 3

For situations where one thread needs to 'signal' another thread to do something, I usually use a System.Threading.ManualResetEvent to signal the secondary thread to stop, like this:

private volatile bool _threadRunning = false;
private ManualResetEvent _signal = new ManualResetEvent(false);
private Thread _thread;
private void OnButtonClick(object sender, EventArgs e)
{
    if (!_threadRunning) {
        // Reset the 'signal' event.
        _signal.Reset();
        // Build your thread parameter here.
        object param = ;
        // Create the thread.
        _thread = new Thread(ExecuteThreadLogicConditionally(param));
        // Make sure the thread shuts down automatically when UI closes
        _thread.IsBackground = true;
        // Start the thread.
        _thread.Start();
        // Prevent another thread from being started.
        _threadRunning = true;
    } else {
        // Signal the thread to stop.
        _signal.Set();
        // DO NOT JOIN THE THREAD HERE!  If the thread takes a while
        // to exit, then your UI will be frozen until it does.  Just
        // set the signal and move on.
    }
}
// If the thread is intended to execute its logic over and over until
// stopped, use this callback.
private void ExecuteThreadLogicUntilStopped(object param)
{
    // Use a while loop to prevent the thread from exiting too early.
    while (!_signal.WaitOne(0)) {
        // Put your thread logic here...
    }
    // Set the flag so anther thread can be started.
    _threadRunning = false;
}
// If the thread logic is to be executed once and then wait to be
// shutdown, use this callback.
private void ExecuteThreadLogicOnce(object param)
{
    // Put your thread logic here...
    //
    // Now wait for signal to stop.
    _signal.WaitOne();
    // Set the flag so another thread can be started.
    _threadRunning = false;
}
// If the thread needs to be stopped at any point along the way, use
// this callback.  The key here is to 'sprinkle' checks of the 'signal'
// to see if the thread should stop prematurely.
private void ExecuteThreadLogicConditionally(object param)
{
    if (_signal.WaitOne(0)) { _threadRunning = false; return; }
    // Execute small chunk of logic here...
    if (_signal.WaitOne(0)) { _threadRunning = false; return; }
    // Execute another small chuck of logic here...
    if (_signal.WaitOne(0)) { _threadRunning = false; return; }
    // Continue this pattern through the method.
}

Note that this solution does not use the ThreadPool at all. It could easily be made to do so. And as a suggestion, I wouldn't muck with SetMaxThreads() function on the ThreadPool. Just let the ThreadPool do its thing. It's been designed to be optimal for the way you use it.