C# Thread.Join() blocks the main thread

Question

I have 3 threads running: the main thread, the readData thread and the acquisition thread. From a form, when the play button is clicked, it starts the device acquisition and the readData threads. When the Stop button is pressed I wanna stop both threads. However, the acqusitionThread.Join() blocks the execution. What am I doing wrong?

Main Form

 private void btnPlay_Click(object sender, EventArgs e)
        {
          daqObj.Start();
        }

        private void btnStop_Click(object sender, EventArgs e)
        {
          daqObj.Stop();
        }

Data acqusition class to read the data from the device

     public void Start()
            {
                _isRunning = true;

            acquisitionDevice.StartAcquisition(); //starts thread for acquisition

                //start data acquisition thread
                _readDataThread = new Thread(readData);
                _readThread.Name = "Read data Thread";
                _redThread.Priority = ThreadPriority.AboveNormal;
                _readThread.Start();
            }

       public void ReadData()
        {

            try
            {
                // write data to file
                while (_isRunning)
                {
                    //Reads data (dequeues from buffer)
                    float[] data = acquisitionDevice.ReadData(numValuesAtOnce);

                    //Do other stuff with data (eg: save to file)
                }
                }
                catch (Exception ex)
                {
                    Console.WriteLine("\t{0}", ex.Message);
                }
    }


            public void Stop()
            {
                _isRunning = false;

                    if ((_writeToFileThread != null) && _writeToFileThread.IsAlive)
                        _writeToFileThread.Join(); //stop readData thread

                    acquisitionDevice.StopAcquisition(); //stops acquisition thread

                     Console.WriteLine("acquisiton thread stopped); //THIS IS NEVER EXECUTED

            }

Device acqusition class:

 public void StartAcquisition(Dictionary<string, DeviceConfiguration> deviceSerials)
        {
            //ensure that data acquisition is not already running
            if (_isRunning || (_acquisitionThread != null && _acquisitionThread.IsAlive))
                throw new InvalidOperationException("Data acquisition is already running!");

            _isRunning = true;

            //initialize buffer
            _buffer = new WindowedBuffer<float>(BufferSizeSeconds * sampleRate * totalChannels);

            //start data acquisition thread
            _acquisitionThread = new Thread(DoAcquisition);
            _acquisitionThread.Name = "DataAcquisition Thread";
            _acquisitionThread.Priority = ThreadPriority.Highest;
            _acquisitionThread.Start(deviceSerials);
        }

 public void StopAcquisition()
        {
            //tell the data acquisition thread to stop
            _isRunning = false;

            //wait until the thread has stopped data acquisition
            if (_acquisitionThread != null)
                _acquisitionThread.Join(); //THIS BLOCKS

            Console.WriteLine("ended"); //THIS IS NEVER EXECUTED
        }

EDIT Instead of a separate thread for reading the data, I do it inside a token cancellation. I use a separate thread for data acquisition form the device (this is needed to continuously get data) and then I read it and write it to a file with a token cancellation.This is the code that works:

public void StartAcquisition()
            {
                // Initialize token
                _cancellationTokenSourceObj = new CancellationTokenSource();
                var token = _cancellationTokenSourceObj.Token;

                    Task.Factory.StartNew(() =>
               {

                   // Start acquisition
                   try
                   {
                       // Write device configuration parameters to .txt file
                       System.IO.StreamWriter file = new System.IO.StreamWriter(deviceConfFilePath);
                       file.WriteLine(gUSBampObj.GetDeviceConfigurationString());
                       file.Close();

                       // create file stream
                       using (_fileStream = new FileStream(daqFilePath, FileMode.Create))
                       {
                           using (BinaryWriter writer = new BinaryWriter(_fileStream))
                           {
                               // start acquisition thread
                               deviceAcquisition.StartAcquisition();

                               // write data to file
                               while (!token.IsCancellationRequested)
                               {
                                   float[] data = deviceAcquisition.ReadData(numValuesAtOnce);

                                   // write data to file
                                   for (int i = 0; i < data.Length; i++)
                                       writer.Write(data[i]);
                               }
                           }
                       }
                   }
                   catch (Exception ex)
                   {
                       Console.WriteLine("\t{0}", ex.Message);
                   }

               }, token)
           .ContinueWith(t =>
           {
               //This will run after stopping, close files and devices here

               // stop data acquisition
               deviceAcquisition.StopAcquisition();

           });


                }
            }

public void StopAcquisition()
{
    _cancellationTokenSourceObj.Cancel();
}

Answer 1

You don't want to block and wait for the other thread to finish. That is what Thread.Join() does.

Instead you would want to perform a thread cancellation. MSDN Managed Thread Cancellation

Answer 2

Thread.Join() is, by design, a blocking call, from the MSDN:

Join is a synchronization method that blocks the calling thread (that is, the thread that calls the method) until the thread whose Join method is called has completed. Use this method to ensure that a thread has been terminated. The caller will block indefinitely if the thread does not terminate.

(Emphasis mine)

So this is by design, since you don't call one of the overloads with a timeout. However your code has another problem, you might not be signaling the thread to terminate like you think.

Thats where the volatile keyword comes in, you should declare your isRunning field with it, like so:

private volatile bool _isRunning;

This will ensure that the compiler does not use single-threaded cache optimizations on the field value, and fetches the latest value each time that field is read. Since you are updating that field from multiple threads, you need to mark it as volatile.

The other problem you have is your while loop:

while (_isRunning)
{
    //Reads data (dequeues from buffer)
    float[] data = acquisitionDevice.ReadData(numValuesAtOnce);

    //Do other stuff with data (eg: save to file)
}

The problem is this line:

float[] data = acquisitionDevice.ReadData(numValuesAtOnce);

If that is a blocking call, and ReadData doesn't return, no matter what you set _isRunning to, it will never terminate the thread until that method returns.

You should look into the Task Parallel Library and cancellable tasks, raw threads are becoming depreciated for higher level control. Also consider async/await instead, since you are doing blocking I/O, really no reason for a new thread to sit and wait for I/O when you can just await an I/O bound task.