7

In my Qt application I create a QThread that should perform some heavy calculation task regularly. Main QApplication thread is supposed to maintain a GUI (not included in example) and perform some regular updates as well. Both threads have their own timers to enable regular update() calls.

Problem: When calculation workload for worker thread exceeds some critical value my main thread stops receiving timer events.

Example code is below. It outputs "Main" when update() is called for main thread, and "Worker" for worker thread. If you run it you'll see that "Worker" is printed regularly and "Main" appears exactly two times (one at the beginning, and one in about 5 seconds). In case of full-featured GUI application this would effectively mean total GUI freeze.

Some observations.

  1. Reducing workload by putting 100 limit on inner cycle (instead of 1000) will fix the problem (both update() methods will be called regularly).
  2. Setting connection type for worker thread timer signal to Qt::DirectConnection will fix the problem.

So, as you can see I have a couple of workarounds on this, but I would appreciate anybody explaining me what's the problem with the original code. I expect threads to execute their event loops independently. I know that I'm blocking the worker thread event loop by a long update() operation but why in the world does it affect the main thread?

P.S. Yes, I know about QConcurrent alternative. But I'd just like to understand.

test.cpp

#include <windows.h>
#include <QApplication>

#include "test.h"

HANDLE mainThread_ = INVALID_HANDLE_VALUE;
QApplication *app_ = 0;
MyObj *obj_ = 0;
MyThread *thread_ = 0;

MyObj::MyObj()
    : timer_(0)
{
    timer_ = new QTimer(0);
    connect(timer_, SIGNAL(timeout()), this, SLOT(update()));
    timer_->start(10);
}

void MyObj::update()
{
    printf("Main\n");
}

void MyThread::run()
{
    timer_ = new QTimer(0);
    connect(timer_, SIGNAL(timeout()), this, SLOT(update()));
    timer_->start(10);

    exec();
}

void MyThread::update()
{
    printf("Worker\n");

    // do some hard work
    float f = 0.f;
    for (int i=0; i < 100000; ++i)
    {
        for (int j=0; j < 1000; ++j)
        {
            f += i * j;
        }
    }
}

int main()
{
    int argc = 0;
    app_ = new QApplication(argc, 0);

    obj_ = new MyObj();
    thread_ = new MyThread();
    thread_->start();

    QApplication::exec();

    return 0;
}

test.h

#include <QTimer>
#include <QThread>

class MyObj : public QObject
{
    Q_OBJECT

public:
    MyObj();

public slots:
    void update();

private:
    QTimer *timer_;
};

class MyThread : public QThread
{
    Q_OBJECT

public:
    void run();

public slots:
    void update();

private:
    QTimer *timer_;
};

UPD: I've got some answers from respectable members (read them below). Now I'd like to clarify what faulty idea broke my code in particular.

As you can see the plan was having two threads each running some update() procedure regularly. My mistake was thinking about update() as just some procedure, and it is a slot. A slot of particular object which has its own thread affinity, meaning that it's body will be executed in that thread (unless signal is dispatched with Qt::DirectConnection). Now, it appears that I've done it all right with timers -- each of them belongs to different thread -- but messed things up with update(). So I ended up executing both update() procedures in the main thread. Apparently at some point event loop becomes flooded with timer events and never finishes the iteration.

As for solutions. If you've read "You're doing it wrong" (and you should indeed) you know that it's rather convenient to have all your logic implemented in an object that is not subclassed from QThread but created separately and attached to QThread with moveToThread(). Personally I see nothing wrong with subclassing from QThread if you keep in mind that your object only controls the thread but doesn't belong to it. So it's not the place for the code you want to be executed in that thread.

ololuki
  • 377
  • 1
  • 7
  • 14

2 Answers2

3

The first problem here is that you're inheriting from QThread, so as it states here, "you're doing it wrong".

The issues you're having stem from thread affinity (which thread an object is running on). For example, if you were to inherit from QThread and create objects in the constructor, without parenting the object, that object will be running in the main thread, not the new thread. So in the MyThread constructor you'd have:-

MyThread::MyThread()
    : timer_(0)
{
    timer_ = new QTimer(0);
    connect(timer_, SIGNAL(timeout()), this, SLOT(update()));
    timer_->start(10);
}

The timer (timer_) here will be running on the main thread, not the new thread. To save repeating myself, one of my previous answers explains thread affinity here.

The best way to solve the problem is to change your class to inherit from QObject and then move that object to a new thread.

TheDarkKnight
  • 27,181
  • 6
  • 55
  • 85
  • Thank you. Actually I've read all these posts like "you're doing it wrong" several times =) But only now I'm starting to get it. So my MyThread object resides in main thread and timer events sent to it get into main thread's event loop. Cool. Now, I wonder why main thread's update() almost never gets an opportunity? I mean both timers have equal intervals, they should go head to head. – Anton Zherzdev Oct 22 '13 at 13:57
  • You can print the address of the thread from both MyObject and MyThread at various points, such as the constructor, the run and update functions. This will give you a better idea of what's going on with respect to thread affinity Just call printf("0x%x", obj->thread()); But overall, if you just refactor and don't inherit from QThread, you'll likely see it just work as expected. – TheDarkKnight Oct 22 '13 at 14:23
  • As an addition to the "You're doing it wrong" article, this is a great follow up on how to use threads: http://mayaposch.wordpress.com/2011/11/01/how-to-really-truly-use-qthreads-the-full-explanation/ – TheDarkKnight Oct 22 '13 at 14:44
  • I was just about to mark your post as the answer but careful reading reveals a bit of misunderstanding. You've put it so that I've got things messed with timer's affinity but in fact that is not the case. My timers are OK. It's MyThread::update() slot that was misplaced. I thought it would be executed in thread timer belongs to, but naturally it is executed in thread MyThread belongs to (the main one). – Anton Zherzdev Oct 22 '13 at 19:52
  • 2
    @AntonZherzdev You should never subclass QThread if you are planning to use slots. As you noticed the slots will be executed in the thread your `MyThread` object lives in, not in the thread it is managing. It is [stated in the docs](http://qt-project.org/doc/qt-5.0/qtcore/qthread.html#details) as well. `implementing new slots in a QThread subclass is error-prone and discouraged` – thuga Oct 23 '13 at 06:57
  • @AntonZherzdev, you mean the update function that's called via the Timer's timeout signal?! My answer is explaining that the creation of objects have a thread affinity tied to the thread they're created on, unless they are moved and by inheriting QThread, it can be misleading as to an object's thread affinity. This is a common problem and unless you want to change how threads are controlled, you should not be inheriting from QThread, as I mentioned in the last paragraph. – TheDarkKnight Oct 23 '13 at 07:53
  • Merlin069, yes, I understand where the problem comes from, now =) Thank you. My concern is that your post can be a bit misleading for other potential readers. Not because your idea is wrong but because you're citing section of my code that isn't there (or at least it looks like that). I just thought you might consider editing your post to be more precise. Or I could write my own... – Anton Zherzdev Oct 23 '13 at 08:39
  • @AntonZherzdev, ok thanks for clarifying. I've edited the post to show more clearly that it's an example and not directly what you posted. – TheDarkKnight Oct 23 '13 at 08:44
2

first of all ... http://blog.qt.digia.com/blog/2010/06/17/youre-doing-it-wrong/

from your main i an see nothing like a GUI ... you just call the QApplication::exec() for whatever reason not app_->exec() (?)

for your threading problem: you can create a QObject derived class that has the slot doUpdate() or so. with this you can do it like:

TheUpdateObject* obj = new TheUpdateObject;
obj->moveToThread(thread_);  // thread_ is a QThread object
thread_->start();
connect(thread_, SIGNAL(finished()), obj, SLOT(deleteLater()));
QTimer* tmr = new QTimer(this);
tmr->setTimeout(10);
connect(tmr, SIGNAL(timeout()), obj, SLOT(doUpdate()));
connect(tmr, SIGNAL(timeout()), tmr, SLOT(start()));
tmr->start();

so the timer should restart itself and the doUpdate() should be called in the other thread.

inside the GUI you do not need to check for updates, the qt framework should redraw when needed.

Zaiborg
  • 2,492
  • 19
  • 28