exec of dialog not return in multithreading environment

tmp15711

What does the code try to do?
Create two worker threads which request the GUI thread to show a dialog.

The worker threads are created by QtConcurrent::run. Then a QObject of type Sender will be instantiated and later on it will send a signal to the GUI thread which is represented by a AppProxy instance.

What is the problem?
When only one worker thread is created, everything is fine. However, the program blocks with two worker threads.

Let's suppose the first dialog created is dialog. dialog.exec() does not return even after the dialog is closed.

Sample Code

#include <QtConcurrent>
#include <QMessageBox>
#include <QDebug>
#include <QApplication>
#include <QMutex>
#include <QMutexLocker>

QMutex qDebugMutex;
#define TRACE()\
    do{\
        QMutexLocker _(&qDebugMutex); Q_UNUSED(_)\
        qDebug() << QThread::currentThreadId() << ":" << __LINE__;\
    }while(false)


struct AppProxy: QObject{
    Q_OBJECT
public:
    static AppProxy& instance(){
        static AppProxy appProxy;

        return appProxy;
    }
    static void createInstance(){
        instance();
    }
public slots:
    void showDialog(){
        TRACE();
        while(dialogShown) qApp->processEvents();
        TRACE();

        dialogShown = true;
        TRACE();
        QMessageBox::information(nullptr, "info", "dialog shown");
        TRACE();
        dialogShown = false;
    }

private:
    bool dialogShown;

    explicit AppProxy(QObject *const parent = nullptr): QObject(parent), dialogShown(false){}
};


struct Sender: QObject{
    Q_OBJECT
public:
    explicit Sender(QObject *const parent = nullptr): QObject(parent){
        QObject::connect(
            this, SIGNAL(showDialog()),
            &AppProxy::instance(), SLOT(showDialog()),
            Qt::BlockingQueuedConnection
        );
    }

    void doSomething(){
        TRACE();
        emit showDialog();
        TRACE();
    }
signals:
    void showDialog();
};


void foo(){
    TRACE();
    Sender sender;
    sender.doSomething();
    TRACE();
}

void bar(){
    TRACE();
    QtConcurrent::run(QThreadPool::globalInstance(), foo);
    TRACE();
}

void wait(){
    TRACE();
    while(!QThreadPool::globalInstance()->waitForDone(50)) qApp->processEvents();
    TRACE();
    qApp->quit();
    TRACE();
}

int main(int argc, char *argv[]){
    QApplication a(argc, argv);
    a.setQuitOnLastWindowClosed(false);

    AppProxy::createInstance();

    QTimer::singleShot(0, bar);
    QTimer::singleShot(0, bar);
    QTimer::singleShot(0, wait);

    a.exec();
    TRACE();
}

#include "main.moc"

Sample output

0x1c90 : 76
0x1c90 : 78
0x16c4 : 69
0x1c90 : 76
0x16c4 : 59
0x1c90 : 78
0x1c90 : 82
0x1be8 : 69
0x1be8 : 59
0x1c90 : 29
0x1c90 : 31
0x1c90 : 34
0x1c90 : 29

Platform
Qt Creator 4.4.1 Based on Qt 5.9.2 (MSVC 2015, 32 bit)
MSVC 2017 (64-bit)
The problem can also be reproduced on Qt5.4 with GCC 4.9.1

SGaist

Hi,

Your design seems pretty convoluted. It's no very usual to try to show a blocking dialog from several threads.

What exactly are you trying to achieve ?

tmp15711

Do some time-consuming work in worker threads
At some time, the worker threads will ask for user input.
The dialogs should not appear at the same time.

kshegunov

Just say "no!" to global objects; and the locking of the qDebug() is just superfluous. You're overengineering the solution, here's some food for thought:

class JobController : public QObject
{
    Q_OBJECT

public:
    JobController(QObject * parent = nullptr)
        : QObject(parent), jobsActive(0)
    {
    }

public slots:
    void showDialog()
    {
        QMessageBox::information(nullptr, "info", "dialog shown");
    }

    // Thread safe for convinience
    template <class Functor>
    void startJob(Functor functor)
    {
        jobsActive++;
        QTimer::singleShot(0, this, [this, functor] () -> void  {
            QtConcurrent::run(QThreadPool::globalInstance(), std::bind(functor, this));
        });
    }
    
    // Thread safe for convinience
    void finishJob()
    {
        if (--jobsActive <= 0)
            QTimer::singleShot(0, qApp, &QCoreApplication::quit);
    }

private:
    QAtomicInt jobsActive;
};

void job(JobController * controller)
{
    // Do whatever you want to do in the thread
    // ...
    // Show the dialog
    QMetaObject::invokeMethod(controller, "showDialog", Qt::BlockingQueuedConnection);
    // Do more stuff
    // ...
    // Finished with the job, notify the controller
    controller->finishJob();
}

int main(int argc, char *argv[])
{
    QApplication app(argc, argv);
    a.setQuitOnLastWindowClosed(false);

    JobController controller(&app);

    // ... Do as many times as needed
    controller.startJob(job);
    controller.startJob(job);

    QObject::connect(&app, &QCoreApplication::aboutToQuit, [] () -> void  {
        QThreadPool::globalInstance()->waitForDone();
    });

    return QApplication::exec();
}

tmp15711

Firstly, your code won't compile without edit, e.g. template methods cannot be slots.
Dialogs are not shown exclusively. Once I add shownDialog and event processing in your showDialog method, it will show only one dialog as well. That is the problem exists.
The problem doesn't exist on Linux, at least not on centos-qt5.4.

kshegunov

@tmp15711 said in exec of dialog not return in multithreading environment:

Firstly, your code won't compile without edit, e.g. template methods cannot be slots.

It's just a typo that I have not added the public: access specifier. I don't compile and test example code I post here ordinarily, it's up to you to do it. startJob and finishJob are not intended as slots in any case.

Dialogs are not shown exclusively. Once I add shownDialog and event processing in your showDialog method, it will show only one dialog as well. That is the problem exists.

To show anything on the screen you must spin the event loop! In fact the best thing to do is to drop the whole idea that you will drive the UI imperatively (this isn't DOS/UNIX) and switch to asynchronous processing - that is split the job in two parts - one before the needed data, and one after the data is obtained and use signals and slots to tie it up together.
For example a workflow like this:
do work (in the worker thread) -> need data (from the worker thread to the UI) -> get data (in the UI thread) -> data obtained (from the UI to the worker thread) -> continue work with the data (in the worker thread).

The problem doesn't exist on Linux, at least not on centos-qt5.4.

This means very little as there are differences in how the UI is integrated with the system event loop and windowing system for each platform.

PS.

If you really, really insist on having the dialogs exclusively you need to serialize the posting of the message to the UI event loop between the workers. Something along the lines of:

    // ... Code from above
    void showDialog()
    {
         // ...
    }

    void requestShowDialog()
    {
         static QMutex workerMutex;
         QMutexLocker locker(&workerMutex);

         QMetaObject::invokeMethod(this, "showDialog", Qt::BlockingQueuedConnection);
    }
    // ...

void job(JobController * controller)
{
    // Do whatever you want to do in the thread
    // ...
    // Show the dialog
    controller->requestShowDialog();
    // Do more stuff
    // ...
    // Finished with the job, notify the controller
    controller->finishJob();
}

JonB

@kshegunov said in exec of dialog not return in multithreading environment:

drive the UI imperatively (this isn't DOS/UNIX) and switch to asynchronous processing

Nothing about UNIX prevents you driving stuff asynchronously rather than synchronously --- the proof is that Qt asynchronous runs under Linux --- just saying! :) [DOS is a different matter.]

kshegunov

@JNBarchan said in exec of dialog not return in multithreading environment:

the proof is that Qt asynchronous runs under Linux

Also under windows. The point is what you do in console applications is (usually) not. The typical console application consists of:

Read input from the command line or the terminal
Do processing very typically in imperative manner
Output results

What you have with GUI applications is event driven input. You don't just read some file, but respond to the user clicking a button or writing something in a text field or w/e.

JonB

@kshegunov
I don't disagree, and your analysis is good. But you lumped UNIX with DOS, and it's the approach not the capabilities which is at issue. UNIX has always had calls to support "asynchronous" rather than "imperative" programming. Sorry, but I leap to UNIX's defence at any hint of "insults" ;-)

kshegunov

@JNBarchan said in exec of dialog not return in multithreading environment:

and it's the approach not the capabilities which is at issue

Okay, I'll grant you that. I myself am a Linux user for a long time, but in any case the issue here is the way the GUI is assumed to need to work by the OP, i.e. imperatively, which is not how it does.

JonB

@kshegunov
Of course I understand. BTW, for my part I am a very new Linux user but a very old (unfortunately) UNIX user! :)

tmp15711

@kshegunov

do work (in the worker thread) -> need data (from the worker thread to the UI) -> get data (in the UI thread) -> data obtained (from the UI to the worker thread) -> continue work with the data (in the worker thread).

I think my approach indeed obeys the idea. The worker thread does some work first, then emits a signal requesting user input, and continues. The only difference is the worker thread doesn't have slots and event loops. If it had, then the GUI thread have to emit a signal for each request, and the threads will tightly coupled which I'd like to avoid.

void requestShowDialog()

This approach has a drawback, namely job cannot be called by the GUI thread. But I will admit that it is a good approach.

This means very little as there are differences in how the UI is integrated with the system event loop and windowing system for each platform.

Obviously, different system has different features. That is why I choose QT. Without declarations, it is resonable to assume the same appearances.

So, is it a bug of QT?

kshegunov

@tmp15711 said in exec of dialog not return in multithreading environment:

The only difference is the worker thread doesn't have slots and event loops.

Thus it's not event driven and hence you need to resort to tricks to tie imperative code to event driven GUI code. The moment you need to block in the middle of your code to wait for something in the GUI to happen, is the moment you should realize you're doing something very odd or wrong.

This approach has a drawback, namely job cannot be called by the GUI thread.

The job can't be called from the GUI thread anyway, because of the Qt::BlockingQueuedConnection. If you call something with that flag from the GUI thread and the receiver is in the GUI thread (which it is) you'd get very simply a deadlock.

Without declarations, it is resonable to assume the same appearances.

Not when it comes to the internals. Something working on one platform and not on the other, doesn't mean it works in principle and there's a problem with the specific platform, quite the contrary it means it doesn't work in principle and by some chance it works on specific platforms.

So, is it a bug of QT?

No!

tmp15711

@kshegunov

Not when it comes to the internals. Something working on one platform and not on the other, doesn't mean it works in principle and there's a problem with the specific platform, quite the contrary it means it doesn't work in principle and by some chance it works on specific platforms.

I cannot agree. Maybe my approach is odd (and if it is, I'd like to admit), but if it is not forbidden anywhere in Qt's documents, it should work.
In fact, the program is not really blocked. If you add a periodic timer, it works all the time. Why the timeout is responsed while exec does not return? Surprisingly.

SGaist

Because exec spins a local event loop which will process events in the context of the dialog.

tmp15711

@kshegunov

Thus it's not event driven

In my opinion, event driven means one direction dependences, i.e. the slots know the signal. If the GUI thread has to response with signals, then it really knows the worker thread. And it should not.

tmp15711

@SGaist Yes, I know that. So? exec should not return when I closing the dialog?

kshegunov

@tmp15711 said in exec of dialog not return in multithreading environment:

I cannot agree.

Let me put it in another way then. I'm a physicist, so I'll give you an example with a simple process:

Suppose you claim that water boils at exactly 100 degrees centigrade, and you measure it 5 times and you confirm that supposition. Now suppose I decide to replicate your experiment but the atmospheric pressure in my lab is lower than in yours. I measure and find out that water boils at 95 degrees, instead of the 100 you claim. It's enough for me to show a measurement (assuming we all agree the measurements are correct) that contradicts your theory to disprove it. It's needed, formally speaking, for you to show an infinite amounts of measurements that support the theory to prove it.

It's by this peculiarity of the scientific method that we learn and "prove" new things. So back to business, your code (which is user code) can't be claimed to work if it doesn't work at all times on all platforms, on the other hand it clearly does not work if it doesn't run even on one platform. And since we can say Qt's signal-slot mechanism and threading is mostly proven (in the sense of the countless test and programs that run it without issue), then the problem must be in the way you wrote the user code.

Maybe my approach is odd (and if it is, I'd like to admit), but if it is not forbidden anywhere in Qt's documents, it should work.

Assuming you implement it correctly, which you had not.

In fact, the program is not really blocked. If you add a periodic timer, it works all the time. Why the timeout is responsed while exec does not return?

See @SGaist's post.

If the GUI thread has to response with signals, then it really knows the worker thread. And it should not.

This is what event driven means. The GUI "notifies the interested parties" that something "interesting" has happened. Whether or not it knows about the workers is another matter that is connected to coupling, but lets not dig into it here. The "event driven" part is that the GUI does that notification at an unspecified time (from the viewpoint of the subscribers). It can be immediately, it can be an hour late, it may even not happen at all.

J.Hilk

@kshegunov thumbs up for a fellow physicist ;-)

@tmp15711 I think you're approaching this from the wrong direction. QtConcurrent is meant more as an fire and forget type of thread:
Something along the line of:

Yo QtConcurrent, calculate Pi to 400 decimals while I watch this video

I would recommend moving your whole stuff in its own class, and move that in a QThread object, that way you can easly react to and request Userinput, if you define suitable signals and slots.

QThread & worker, general usage

tmp15711

@kshegunov Ok. Really, your words are powerful. Then, if my code is wrong, I want to know the key error, a formally logical analysis. For example, if we have a deadlock, we would say: oh, you try to lock a mutex that has been locked by some other thread which in turn tries to lock the mutex you have taken.