How to stop thread - Qthread

Question

I have to start/stop a thread on press of two different buttons.
Please suggest my code is correct or not. Do I have missed something in the connect() call ?

Problem i am facing is that after calling quit() on my thread, then i wait for my thread to finish, but wait() call on thread never returns true, and my program is stuck in , while(!m_deviceThread.wait())

Please suggest how to resolve this ?

My devicethread & worker object defined in Mainwindow class :--

QThread m_deviceThread;
deviceThreadObject *m_deviceThreadObject;

Main device thread object :-----

class deviceThreadObject : public QObject
{
    Q_OBJECT
public:
    explicit deviceThreadObject(QObject *parent = 0);

    /*!
      Termination control main thread
    */
    bool m_bQuit;


signals:

public slots:
    void dowork();

};

deviceThreadObject object constructor :--

// constructor for the deviceThreadObject
deviceThreadObject::deviceThreadObject(QObject *parent) :
    QObject(parent)
{
    m_bQuit = false;
}

I have main thread m_deviceThread which runs on button start pressed:---

void MainWindow::on_actionStart_triggered()
{
    if(!b_threadAlreadyStarted)
    {

   m_deviceThreadObject = new deviceThreadObject;

    // connect device thread signal to the slot
    connect(&m_deviceThread ,SIGNAL(started()),m_deviceThreadObject,SLOT(dowork()));

    m_deviceThreadObject->moveToThread(&m_deviceThread);

    // Set the flag before starting slot of main thread
    m_deviceThreadObject->m_bQuit = true;

    m_deviceThread.start();

    }
}

I have main thread m_deviceThread which stops on button stop pressed:---

void MainWindow::on_actionStop_triggered()
{
    if(b_threadAlreadyStarted)
    {

        b_threadAlreadyStarted = false;

        // get out of event loop
        m_deviceThreadObject->m_bQuit = false;

        m_deviceThread.quit();
        qDebug() << " \n quit ";

        // wait for thread to terminate
        while(!m_deviceThread.wait());

        m_deviceThreadObject->deleteLater();

        qDebug() << " \n finished";

    }

}

// Common slot for the Device - thread

void deviceThreadObject::dowork()
{

  while(m_bquit)
  {

     // Do some work

  }

}

Answer 1

I've seen a few different ways to do this in the past.

Below is showing what the compiler optimizes your code to look like:

bool quit = false;
while(!quit)
{
    // no reference to quit here or in any functions mentioned in here
}

could get turned into just a forever loop.

// bool quit = false
while(true)
{
    // looks the same to the compiler...
}

The best practice that forces you to know thread synchronization and critical areas or mutexs or semaphores, is to treat the access to the quit parameter as a variable shared between threads, and wrap it so it is only accessed by one thread at a time. My preferred method is to use a QMutexLocker, since it handles well with scope changes, returns, breaks, etc.

http://qt-project.org/doc/qt-5.1/qtcore/qmutexlocker.html#details

So your code gets the additions like this:

deviceThreadObject::deviceThreadObject(QObject *parent) :
    QObject(parent)
{
    m_mutex = new QMutex();
}

void deviceThreadObject::stop()
{
    QMutexLocker locker(m_mutex);
    m_stopped = true;
}

void deviceThreadObject::doWork()
{
    m_stopped = false;
    while(true)
    {
        // The core work of your thread...

        // Check to see if we were stopped
        {
            QMutexLocker locker(m_mutex);
            if(m_stopped)
                break;
        }// locker goes out of scope and releases the mutex
    }
}

A quick and dirty shortcut is to use a volatile bool. It is not a recommended practice, and isn't as reliable across compilers.

Hope that helps.