5

I'm trying to find a better understanding of Qt signals and slots in conjunction with threads. So I tried this minimal application:

foo.h:

#include <QObject>

class A : public QObject {
  Q_OBJECT

public:
  void doit();

signals:
  void x();
};

class B : public QObject {
  Q_OBJECT

public slots:
  void h();
};

foo.cpp:

#include "foo.h"

#include <QThread>
#include <QCoreApplication>

void B::h() {
  qDebug("[%d] B::h() here!", (int) QThread::currentThreadId());
  QCoreApplication::instance()->quit();
}

void A::doit() {
  qDebug("[%d] emitting...", (int) QThread::currentThreadId());
  emit x();
}

int main(int argc, char* argv[]) {
  QCoreApplication app(argc, argv);
  A a;
  B b;
  QObject::connect(&a, SIGNAL(x()), &b, SLOT(h()));
  QThread t;
  t.start();
  b.moveToThread(&t);
  a.doit();
  t.wait();
  return 0;
}

Everything is fine, only the t.wait() at the end never returns. My understanding is calling quit() should stop the event loop, which means exec() should return and so should run() and thread execution should stop. Am I missing something?

Elektito
  • 3,863
  • 8
  • 42
  • 72
  • 3
    the name of your methods should explicit their purpose. – UmNyobe Feb 20 '12 at 09:24
  • Don't worry, this is not production code! I find phony names work much better in short sample/test code, than artificially descriptive names. – Elektito Feb 20 '12 at 09:33
  • I agree with UmNyobe. It would be easier to read and understand the sample code if you used more informative names. E.g. A::doit() -> A::emitThreadStart(), void x() -> startThread(), void h() -> void quitApplication() ... etc – Dmitriy Feb 20 '12 at 11:56
  • Perhaps you're right, since this was ultimately posted on a public forum. I like to use short names in my own tests, but this might not be the place for it. – Elektito Feb 20 '12 at 13:07

2 Answers2

11

QCoreApplication::quit () is not stated as thread-safe method so you can't call it from another thread. Your application can crash or get an undefined behavior(UB).

t.wait() will never return because the running thread is constantly waiting for events. To stop the thread you must call QThread::quit () [slot]

If you want to quit the application after the work is done you have to emit a signal, which is connected to QCoreApplication::quit () [static slot]

If you want to stop the worker thread after the work is done you also have to emit a signal, connected to void QThread::quit () [slot]

Added: Threads, Events and QObjects for further reading

Important notice: You must call QCoreApplication::exec() in order to be able to use signal & slot mechanism between threads, queued connections.

From Qt QThread doc:

Each QThread can have its own event loop. You can start the event loop by calling exec(); you can stop it by calling exit() or quit(). Having an event loop in a thread makes it possible to connect signals from other threads to slots in this thread, using a mechanism called queued connections. It also makes it possible to use classes that require the event loop, such as QTimer and QTcpSocket, in the thread. Note, however, that it is not possible to use any widget classes in the thread.

Doc for Qt::QueuedConnection:

The slot is invoked when control returns to the event loop of the receiver's thread. The slot is executed in the receiver's thread.

Dmitriy
  • 5,357
  • 8
  • 45
  • 57
  • You are right. I'm not stopping the correct event loop. Unfortunately, UmNyobe pointed this out before you, so I have to accept their reply. I'm upvoting your reply though, since it's clearer. Thank you. – Elektito Feb 20 '12 at 09:37
  • On second thought, I think I'd better accept your answer, since the other one has a lot of misinformation and can be rather confusing for other people reading it. – Elektito Feb 20 '12 at 09:40
  • About your added notice, in my code snippet, I haven't called `QCoreApplication::exec()` but the signals/slots do work. Seems to me that the event loop inside the QThread suffices. – Elektito Feb 20 '12 at 16:04
  • That's because you don't post events from your thread to the core application. Try to emit a signal from the thread, which is connected to the application's slot. – Dmitriy Feb 20 '12 at 17:01
  • My understanding is, I only need an event loop where I want to have slots. Since I don't need to handle signals in the main thread of my application (the one I'm working on really, not just this simplified sample), one event loop in the worker thread will do just fine. So in short, `QCoreApplication::exec()` is not always necessary as you've said it is. – Elektito Feb 20 '12 at 21:05
  • If you want your application's main thread not to handle events than you certainly don't need to call exec. – Dmitriy Feb 20 '12 at 21:55
2

It seems that there are many things wrong with your code.

  • You don't call app.exec(). Meaning that there is no main event loop. The x signal of A will not be emitted.
  • By default, a thread has it's own even loop in Qt (at least since few years). Then starting a thread call Qthread::run(), and the event loop is started. That's where your thread is, not in t.wait().
  • What is the purpose of t.wait()? I believe you are misusing it.
  • (If everything else was fine), in B::h() you are stopping the main thread from the other thread. Is that what you wanted to do?

So my first advice will be to add app.exec(), see how it behaves. Explain what your are trying to do, and rewrite something else. Because you're doing it wrong

UmNyobe
  • 22,539
  • 9
  • 61
  • 90
  • t.start() calls the run() method whose default implementation calls exec(). As I said, the signals _are_ emitted and received correctly. wait() is the equivalent of pthread_join, or so the documentation says, so all I'm doing is to wait for the thread to finish. Finally, your last point is correct. I'm not stopping the thread event loop. Changing that line to `QThread::currentThread()->quit()` does the trick. Thank you. – Elektito Feb 20 '12 at 09:31
  • exec() keeps running so wait() has no effects. – UmNyobe Feb 20 '12 at 10:02
  • That's true of course, but the intent of the wait() is to wait for the end of exec(), so using wait() is not wrong per se; not stopping the right event loop is. – Elektito Feb 20 '12 at 10:14
  • Since I'm not using the main event loop, do I really need app.exec()? Seems like everything works fine without it. – Elektito Feb 20 '12 at 13:09