exec of dialog not return in multithreading environment



  • 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


  • Lifetime Qt Champion

    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 ?



    • 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.


  • Qt Champions 2016

    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();
    }
    


    • 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.


  • Qt Champions 2016

    @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();
    }
    


  • @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.]


  • Qt Champions 2016

    @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:

    1. Read input from the command line or the terminal
    2. Do processing very typically in imperative manner
    3. 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.



  • @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" ;-)


  • Qt Champions 2016

    @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.



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



  • @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?


  • Qt Champions 2016

    @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!



  • @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.


  • Lifetime Qt Champion

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



  • @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.



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


  • Qt Champions 2016

    @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.



  • @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



  • @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.



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

    I'm a physicist,

    [ Ah! OT, sorry, but where can I post on this forum to ask you about my (layman's) theories on quantum mechanics? ;-) ]


  • Qt Champions 2016

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

    Then, if my code is wrong, I want to know the key error, a formally logical analysis

    Okay, I'll bite. Since @SGaist duly noted that QDialog::exec spins the event loop of the GUI thread, which it most certainly does, if you use only:

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

    That means that while you're waiting for input in the dialog in the background signals and slots are still fired. This implies also that your second thread calling the above function will cause the GUI to open a second dialog. Now to prevent this I've added a serialization primitive (the mutex) which ensures that only one of the worker threads can post the showDialog call to the UI event loop. When one of the workers finally continues, only then another worker can request a dialog to be shown.

    where can I post on this forum to ask you about my (layman's) theories on quantum mechanics?

    That'd be The Lounge, but don't hold your breath, no one really understands it (it's a math theory anyway).



  • @J.Hilk Then what if QtConcurrent encounters a problem or meets an error? Shouldn't QtConcurrent ask for help and wait for a moment? If I can use Qt::BlockingQueuedConnection, why use slots? I don't need an event loop and slots are really cumbersome in this case.


  • Qt Champions 2016

    You can use the returned from QtConcurrent::run future (QFuture) to signal errors or return data that's been accumulated during the execution of the operation.



  • @kshegunov You are correct. I have tested. The mutex approach certaintly works. But why not mine, the event loop approach? I mean why exec doesn't return after the dialog is closed.

    @kshegunov Error doesn't mean the threads have to exit. Or, maybe just a warning.


  • Qt Champions 2016

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

    But why not mine, the event loop approach? I mean why exec doesn't return after the dialog is closed.

    Because both of the slots are executed and you have interleaved execution, due to the fact that exec() will spin the even loop. Think of it like this:

    1. Thread 1 calls QDialog::exec this causes the even loop to continue receiving events
    2. Thread 2 calls QMetaObject::invokeMethod(this, "showDialog", Qt::BlockingQueuedConnection); which posts an event to the event loop. Due to 1) that slot starts to execute and a second dialog and a nested QDialog::exec is called.

    That's why the mutex works, it ensures that only one QDialog::exec is called at any one point by serializing the calls to QMetaObject::invokeMethod.

    Error doesn't mean the threads have to exit. Or, maybe just a warning.

    Then consider using class deriving from QRunnable and QObject to represent your job, and to so you can have signals in it to tell the GUI some error/warning has occurred. Alternatively, I'd suggest following @J-Hilk's advice and just switching to plain old QThread classes with worker objects.



  • @kshegunov Thanks. I can reproduce the same behavior with a single thread.

    #include <QtCore>
    
    struct Object: QObject{
    	Q_OBJECT
    public slots:
    	void foo(){
    		qDebug() << __LINE__;
    		while(entered) qApp->processEvents();
    
    		qDebug() << __LINE__;
    		QEventLoop loop;
    		QTimer::singleShot(1000, &loop, SLOT(quit()));
    		entered = true;
    		loop.exec();
    		entered = false;
    		qDebug() << __LINE__;
    	}
    
    private:
    	bool entered = false;
    };
    
    int main(int argc, char *argv[])
    {
    	QCoreApplication a(argc, argv);
    	Object w;
    
    	QTimer::singleShot(0, &w, SLOT(foo()));
    	QTimer::singleShot(0, &w, SLOT(foo()));
    	QTimer::singleShot(3000, &a, SLOT(quit()));
    
    	a.exec();
    	qDebug() << __LINE__;
    }
    
    #include "main.moc"
    

    In fact, it has nothing to do with multi-threads. I have never studied Qt source code. It seems exec will not return before all slots finish. So, the code actually creates a dead lock.


Log in to reply
 

Looks like your connection to Qt Forum was lost, please wait while we try to reconnect.