0

I have looked at all the solutions for this topic but still cannot seem to accomplish the stopping of a thread without using Thread.Abort(). Here is the code: Main class code that creates the the thread:

            _pollingThread = new Thread(pollingThread);
            _pollingThread.Start();

And here is the thread:

 void _pollingThread()
 {
     while (continuePolling) //initially set to true
     {
          //do some things can take about 200ms to run
     }
 }

I next attempt to stop the thread from the main thread by setting continuePolling to false.

 private void form1_FormClosing(object sender, FormClosingEventArgs e)
 {
     continuePolling = false;
     Thread.Sleep(1000);

     _pollingThread.Join(1000);

    if (_pollingThread.IsAlive) //always alive!
    {
        _pollingThread.Abort;
    }
 }

Can someone tell me what I am doing wrong? Thanks

Tom
  • 527
  • 1
  • 8
  • 28
  • Where and how is `continuePolling` defined? I don't think the code you've given is enough to replicate the problem you having. – Matan Shahar Apr 11 '18 at 16:48
  • 1
    Don't abort a thread, use a synchronization primitive to tell it to go away. – xxbbcc Apr 11 '18 at 16:48
  • or use thread.interupt – pm100 Apr 11 '18 at 16:55
  • thread.Join waits for the thread to finish. You will hang there. You need to tell your thread to stop by setting continuePolling to false – pm100 Apr 11 '18 at 16:56
  • @pm100 only use exceptions when you don't expect them – Jeroen van Langen Apr 11 '18 at 16:56
  • Remove `Thread.Sleep(1000)`, it is not necessary because you are using `Thread.Join() `. The thread `_pollingThread ` will finish until the execution of method `_pollingThread` has been finished. So, it's important to check how often the loop `while (continuePolling)` is evaluated. So, it's important that you post the full code of `_pollingThread`. – Jorge Omar Medra Apr 11 '18 at 16:57
  • continuePolling is defined as volatile bool continuePolling = true; – Tom Apr 11 '18 at 17:03
  • You'd better not use `volatile` read this: https://stackoverflow.com/questions/72275/when-should-the-volatile-keyword-be-used-in-c – Jeroen van Langen Apr 11 '18 at 17:04
  • I cannot post the full code. It's just too much. However, it takes less than 100ms to run. – Tom Apr 11 '18 at 17:04
  • If you want to `Join` a thread with a timeout, you should check the result (bool) of the `Join` instead of checking the `IsAlive` property. – Jeroen van Langen Apr 11 '18 at 17:08
  • @J. van Langen I checked and it is false. So the join never works. – Tom Apr 11 '18 at 17:15
  • If Join returns true, the thread is finished. The alive property might give a true, because internally the join is waiting on an event also and when it is triggered the thread is still closing. So the alive might give true as result. When the Join is ready, none of your code is executed anymore. The Alive isn't relevant in this purpose. It would be better to check the ThreadState. – Jeroen van Langen Apr 11 '18 at 17:35
  • My guess is that your thread is, at some point, invoking something synchronously on the GUI thread (or waits on the GUI thread through some other means). That effectively deadlocks the program, because the GUI thread is busy joining the thread. – dialer Apr 11 '18 at 17:40
  • Replace `Thread.Join(1000)` with just `Thread.Join()`, and then run the program. When you notice it's deadlocked, press the PAUSE button in visual studio, and examine what the background thread is doing. – dialer Apr 11 '18 at 17:41

2 Answers2

2

Using Abort/Interrupt to stop a thread, is bad programming. You never knew what it did and what it didn't. There is one exception for that (like terminating hanging 3rd party code), even then consider it evil. You should use the ManualResetEvent to tell the thread to terminate execution. The ManualResetEvent is thread-safe and works great.

Here's an example:

public class MyThread
{
    private ManualResetEvent _terminate = new ManualResetEvent(false);
    private Thread _thread;

    private void PollingThread()
    {
        while(!_terminate.WaitOne(0))
        {
            // do your stuff, if you want a pause after each loop,
            // you should change the 0 of waitone. This way the 
            // termination isn't blocked when waiting
        }
    }

    public MyThread()
    {
        _thread = new Thread(PollingThread);
        _thread.Start();
    }

    public void Stop()
    {
        if(_thread != null)
        {
            _terminate.Set();
            _thread.Join();
            _thread = null;
        }
    }
}
Jeroen van Langen
  • 21,446
  • 3
  • 42
  • 57
  • I just tried this. It makes no difference. The join never occurs. – Tom Apr 11 '18 at 17:22
  • I use the model for years and still works like a charm. Your thread probably hangs in a inner loop or waits on something else. You should debug it. Is there a forever running while in it? It should not.. – Jeroen van Langen Apr 11 '18 at 17:25
  • Well that's interesting...I replaced everything in the thread with a Thread.Sleep(1000) and now it joins correctly. – Tom Apr 11 '18 at 17:31
  • This is very interesting. I have one call to a method in a class that reads a serial port and that call always runs quickly. When I issue a _terminate.Set() or continuePolling = false, this call never returns. So in your example, PollingThread would have a call like data = BackEnd.Get_Data(); This always works fine but the minute there is a _terminate.Set() it never returns. Why? The serial port class is a static class. – Tom Apr 11 '18 at 18:07
  • I have changed direct access to the serial port class to an interface to another class that also access the serial port. No more issues. Thanks for the help. – Tom Apr 11 '18 at 18:28
  • The serialport class probably blocks execution until it receives data. – Jeroen van Langen Apr 11 '18 at 18:38
  • I still recomment to replace the continuePolling with an event. It's way more solid. Another advantage is, that it can 'sleep' the but still can be terminated quickly (with a thread.sleep it cannot) – Jeroen van Langen Apr 11 '18 at 18:38
0

continuePolling has to be declared volatile, otherwise there is no guarantee that modifications in one thread will be seen in any other thread.

https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/volatile

Alternatively you might consider using something like System.Timers.Timer to run your polling action periodically.

Jelaby
  • 1,107
  • 1
  • 9
  • 19