aboutToQuit() signal question

Chris Kawa

Since the signal is emitted in another thread it is delivered via queued connection. These are processed in an event loop of the receiving thread, but since you're in the aboutToQuit() handler there's no event loop running in the main thread anymore.

Ideally you would wait for all the other threads to finish before you quit the event loop in the main thread.
If you can't do that for some reason I would wait for other threads to finish in the aboutToQuit() handler and then call processEvents() manually to process anything these threads posted after you already quit the event loop.

DRoscoe

@Chris-Kawa said in aboutToQuit() signal question:

If you can't do that for some reason I would wait for other threads to finish in the aboutToQuit() handler and then call processEvents() manually to process anything these threads posted after you already quit the event loop.

My comm thread has an event loop of its own and I don't have an event to tell the thread to quit, so there is currently no notion of the thread "finishing". I am simply sending an "EXIT" command through the comm thread to the rest of the system. Should I be doing something differently so I know when the thread is finished?

The documentation says that QCoreApplication::processEvents() only processes events for the calling thread. The aboutToQuit() signal is on my main application thread, so processEvents() would only act on those events. Since the comm thread is emitting the logMessage() call on its thread, should I also be calling processEvents() on that thread?

Chris Kawa

My comm thread has an event loop

If it has a loop then it finishes when it exits that loop. The "EXIT" command is something internal to your app and I know nothing about it, but I suspect it does something like calling quit() on the QThread object which exits its loop.

The aboutToQuit() signal is on my main application thread, so processEvents() would only act on those events

And that's what you want. When you emit a signal in the comm thread connected to an object in the main thread it posts a queued signal event in the main thread's event loop. When the events are processed in the main thread's loop it dispatches that event by calling connected object's slot.
So to process that event you need to call processEvents() in the main thread to dispatch that queued signal. You don't need to call it in the comm thread. There's no events to process there.

The usual workflow is that when you get a reason to exit the app, e.g. closeEvent of your main window, you ask your worker threads to finish (usually by emitting some signal that makes the worker thread call quit() and exit its event loop), then wait for them to obey your request and only then exit your main thread's event loop. Generally the main thread's event loop should be the last to quit. It's not a requirement but that's the easiest model to work with.

kshegunov

@DRoscoe
If you're wondering, here you can see how to quickly wrap the waiting for the workers with an atomic int and a semaphore.

@Chris-Kawa said in aboutToQuit() signal question:

It's not a requirement but that's the easiest model to work with.

It's pretty darn close. Qt will throw warnings at you if you exit without waiting for the workers to quit, and for a good reason. :)

Chris Kawa

@kshegunov said:

It's pretty darn close.

I just meant it's not a requirement that the main thread's event loop quits last. You should of course wait for worker threads to finish, either still in the loop or after it exits.

@DRoscoe here's a visual aid of one way to use worker threads:
Threads

kshegunov

@Chris-Kawa said in aboutToQuit() signal question:

I just meant it's not a requirement that the main thread's event loop quits last.

My bad, I've misread the sentence.

DRoscoe

@Chris-Kawa and @kshegunov Thank you both for your excellent replies. I ran into a new problem trying to implement your advice. When I construct my main class, I am doing the following:

engine_comm_thread = new QThread(this);

Then I am doing:

  connect(eng_comm_worker, SIGNAL(finished()),
          engine_comm_thread, SLOT(quit()));
  connect(eng_comm_worker, SIGNAL(finished()),
          eng_comm_worker, SLOT(deleteLater()));
  connect(this, SIGNAL(killEngineCommThread()),
          eng_comm_worker, SLOT(killThread()));
  connect(engine_comm_thread, SIGNAL(finished()), 
          engine_comm_thread, SLOT(deleteLater()));

The worker object is created and moved to the thread here:

    QHostAddress host_address = getHostAddress();
    eng_comm_worker = new TcpCommunicationHandler(host_address,
                                                  engine_port);
    // move the worker object to the comm handler thread
    eng_comm_worker->moveToThread(engine_comm_thread);

In the aboutToQuit() handler of my main app, I am doing:

    emit killEngineCommThread();
    engine_comm_thread->wait();

I set a breakpoint in my worker class's handler for killEngineCommThread() and it is getting hit correctly. It then does the following:

emit finished();

The problem is that the call to engine_comm_thread->wait() is never returning, indicating that it never hit quit();

kshegunov

Hi,
What happens if you use:

connect(eng_comm_worker, SIGNAL(finished()), engine_comm_thread, SLOT(quit()), Qt::DirectConnection);

?

DRoscoe

@kshegunov That both did and didn't work. The direct connection avoided the block on the ::wait() call, but it also prevented the thread from behaving properly. There is a signal emitted back to the main app that was not being properly delivered and I was losing data. I believe the reason why the ::wait() call was blocking was because there was en event that was being queued for the main app from the comm thread. Since the main thread event loop was stopped, it could never report that it was finished(). This seems to be bolstered by the fact that the following code DOES work:

    // Send an EXIT command to Tap Engine
    TapUserEventMessage* exit_message = new TapUserEventMessage(
                                                      TapRequestAction::EXIT,
                                                        "SHUT_DOWN_USER_EXIT");
    emit aboutToTerminate(exit_message);
    emit killEngineCommThread();
    while (engine_comm_thread->isRunning())
    {
      QCoreApplication::processEvents();
    }

I put a qDebug() statement inside the while loop and I was in fact, getting events dispatched to the main thread, which would not have otherwise been processed. The above code solved my problem, and I was able to avoid direct connections.

Thanks again for all of your input. You and @Chris-Kawa have given me a much better level of confidence that I am cleaning up properly on shutdown, which I was not at all confident about prior.

-Dave

kshegunov

Hi,
If I read the whole signal-slot chain correctly, the problem is that you call QThread::wait and block the (main thread's) event loop before the QThread object is able to process the QThread::quit slot, but I wanted to have confirmation, whence the question.

This is a bit of a peculiarity of Qt's thread control - the thread-controlling object, namely the QThread (a QObjectsubclass), is living outside of the controlled thread. So when you connect your worker object's signal to the QThread::quit slot it is in fact queued through the QThread instance's event loop (in this case the main thread).

As a side note my implementation with the semaphore doesn't suffer that particular deficiency regardless of running event loops, as it is using only direct connections for notifying of threads starting/stopping.

Kind regards.

DRoscoe

Ah right! I forgot about that peculiarity of the Qt object model. So, what you're saying is that while the QThread is managing a unique and separate thread, the QThread object itself is actually operating on the main thread, causing the ::wait() to essentially deadlock.

I'm going to take a longer look at your example. I need an approach that is easier and more intuitive to manage.

kshegunov

@DRoscoe said in aboutToQuit() signal question:

So, what you're saying is that while the QThread is managing a unique and separate thread, the QThread object itself is actually operating on the main thread, causing the ::wait() to essentially deadlock.

Indeed. That's why I usually would just wait() on the workers after QCoreApplication::exec() has returned (assuming I use that approach). Another thing you could also do is to not block the event loop, but attach a helper slot that calls quit() and then waits for the worker to exit (I believe that's what Chris had in mind in the first place):

void MyThreadController::requestQuit()
{
    // This should work in this case, but always keep in mind `QObject::moveToThread` isn't thread-safe
    // so getting the thread of an object is "safe" only from the thread it lives in
    // unless you guarantee that it won't be changed mid-call
    QThread * workerObject = sender()->thread();

    // Calling this immediately will *post* an event to the worker thread's event loop
    // (we haven't returned control to the current event loop so we call this directly)
    thread->quit();
  
    // Waiting for the quit request to be processed.
    thread->wait();
}

And then connect you signal to this particular helper slot (the usual way):

QObject::connect(eng_comm_worker, &WorkerObject::finished, this, &MyThreadController::requestQuit);