Properly terminate a QThread

Question

I've have a worker class that do image acquisition in the background.

void acq::run ()
{
    while (m_started)
    {
        blocking_call();
    }
    emit workFinished();
}

void acq::start ()
{
    m_started = true;
    run();
}

void acq::stop ()
{
    m_started = false;
}

start (); stop () are slots and workFinished is a signal.

So in my UI Class, I launch the worker and I connect the signals to the slots :

m_thread = new QThread;
m_worker = new acq();

m_worker->moveToThread(m_thread);

// When the thread starts, then start the acquisition.
connect(m_thread, SIGNAL (started ()), m_worker, SLOT (start ()));

// When the worker has finished, then close the thread
connect(m_worker, SIGNAL(workFinished()), m_thread, SLOT(quit()));

m_thread->start();

At this point, I implemented the slot, closeEvent

void UIClass::closeEvent (QCloseEvent *event)
{
    m_worker->stop(); // tell the worker to close
    m_thread->wait(); // wait until the m_thread.quit() function is called
    event->accept(); // quit the window
}

Unfortanely, m_thread->wait() is blocking. Even if the signal quit() is emmited

Thanks

edit:

I added these two connections :

connect(m_worker, SIGNAL(workFinished()), m_worker, SLOT(deleteLater()));
connect(m_thread, SIGNAL(finished()), m_thread, SLOT(deleteLater()));

and a Qdebug into acq::~acq()

The message is printed that prove, that stop is called, workFinished is emitted, deleteLater() is emitted.

Answer 1

A normal signal/slot connection between objects on different threads requires that the thread of the receiver object runs an event loop.

Your receiver thread does theoretically run its event loop, but the event loop is busy executing the start() slot because run() never returns.

You either need to unblock the receiver event loop or call the stop slot with a Qt::DirectConnection.

When doing the latter you need to be aware that the slot is now called in the context of the sender thread and you need to protect m_started against concurrent access.

Alternatively to using your own flag you could use QThread::requestInterruption() and QThread::isInterruptionRequested()

Answer 2

Add

QCoreApplication::processEvents();

to your loop and it'll work.

The reason you deadlock is that the call to acq::run() blocks and does not leave time for acq::stop() to be executed on the worker thread.

Answer 3

With the help of Ralph Tandetzky and Kevin Krammer I finally found a solution.

• Instead of closing the thread with m_worker->stop();, I use QMetaObject::invokeMethod(m_worker, "stop", Qt::ConnectionType::QueuedConnection); and QCoreApplication::processEvents(); in the worker event loop. The behavior does not change, but I hope it will prevent race condition or other problems.

• Instead of using : connect(m_worker, SIGNAL(workFinished()), m_thread, SLOT(quit()));, I use a custom slot :

  connect(m_worker, &Acq::workFinished, [=]
  {
      std::this_thread::sleep_for(std::chrono::milliseconds(100));
      QMetaObject::invokeMethod(m_thread, "quit", Qt::ConnectionType::DirectConnection);
  });
  

  We use DirectConnection because we are outside the infinite loop, so the event are not processed.

• With that, I had a last problem. m_thread->wait is blocking, and I've to read events, otherwise, my custom slot will never be called. So added an event loop to my UI Class QEventLoop m_loop.
  Just before m_thread->wait(), I wrote m_loop.exec(); And finally, In my custom slot, I put m_loop.quit()

  connect(m_worker, &Acq::workFinished, [=]
  {
      std::this_thread::sleep_for(std::chrono::milliseconds(100));
      QMetaObject::invokeMethod(m_thread, "quit", Qt::ConnectionType::DirectConnection);
      m_loop.quit();
  });
  

  m_loop.exec() process event until quit m_loop.quit() is called. With that method, I don't even need m_thread->wait() because m_loop.quit() is called when workFinished is emitted. I don't need QMetaObject::invokeMethod(m_thread, "quit", Qt::ConnectionType::DirectConnection); anymore

Now it works like a charm

EDIT: This solution is very heavy and ugly, Qt (https://www.qtdeveloperdays.com/sites/default/files/David%20Johnson%20qthreads.pdf) sugest to use subclass and requestInteruption in my case.