Search code examples
qtqthreadqtconcurrent

Qt: How to avoid deadlock when multiple queued signals invoke same slot


In following code I meet deadlock in someOperation:

class A : public QObject {
    Q_OBJECT
public:
    explicit A(QObject* parent) : QObject(parent), data(0) {}
public slots:
    void slot1() {
        someOperation();
    }
    void slot2() {
        someOperation();
    }
    void slot3() {
        someOperation();
    }
private:
    void someOperation() {
        QMutexLocker lk(&mutex);
        data++;
        QMessageBox::warning(NULL, "warning", "warning");
        data--;
        assert(data == 0);
    }
    int data;
    QMutex mutex; //protect data
};

class Worker: public QThread {
    Q_OBJECT
public:
    explicit Worker(QObject* parent) : QThread(parent) {}
protected:
    virtual void run() {
        // some complicated data processing
        emit signal1();
        // other complicated data processing
        emit signal2();
        // much complicated data processing
        emit signal3();
        qDebug() << "end run";
    }
signals:
    void signal1();
    void signal2();
    void signal3();

};

int main(int argc, char *argv[])
{
        QApplication app(argc, argv);
        A* a = new A(&app);
        Worker* w = new Worker(a);
        QObject::connect(w, SIGNAL(signal1()), a, SLOT(slot1()), Qt::QueuedConnection);
        QObject::connect(w, SIGNAL(signal2()), a, SLOT(slot2()), Qt::QueuedConnection);
        QObject::connect(w, SIGNAL(signal3()), a, SLOT(slot3()), Qt::QueuedConnection);
        w->start();

        return app.exec();
}

There is a thread that will emit three signals, all of them queued connected to an instance of class A, and all class A' slots will call to someOperation, and someOperation is protected by mutex and it will popup a message box.

Qt::QueuedConnection 2 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.

It seems slot2 is invoked when slot1's message box still doing modal, in main thread, but at that time slot1 has lock mutex, so deadlock.

How to change the code to avoid deadlock?

Update:(Jan.17, 2019)

What I want archive is that: slot2 not be execute before slot1 finished.

What should be kept are:

  1. worker is a background thread to process data, cost long time; so, whatever, the three signals will emit from other thread.
  2. worker should not blocked by emitting signals.
  3. slots should execute in main thread, because they will update GUI.
  4. someOperation is not reentrant.

Solution

  • The requirement that "someOperation is not reentrant" is an odd one. What should happen if reentrancy is attempted? Given that someOperation can only be called from the main thread I can only see two options...

    1. Block completely with mutex/barrier etc. as you have tried.
    2. Block based on a recursion level counter and spin the event loop until that counter decrements to zero.

    1) Will block the thread's event loop completely preventing the current message dialog from functioning correctly.

    2) Will allow all message dialogs simultaneously rather then serialising them.

    Rather than trying to make someOperation non-reentrant I think you need to make sure you use in a way that won't result in reentrancy.

    One option might be to make use of a separate QObject derived class instance on its own QThread. Consider the following...

    class signal_serialiser: public QObject {
      Q_OBJECT;
    signals:
      void signal1();
      void signal2();
      void signal3();
    };
    

    If an instance of signal_serialiser is moved to its own thread it can act as a queue to buffer and forward the various signals if suitable connection types are used. In your code you currently have...

    QObject::connect(w, SIGNAL(signal1()), a, SLOT(slot1()), Qt::QueuedConnection);
    QObject::connect(w, SIGNAL(signal2()), a, SLOT(slot2()), Qt::QueuedConnection);
    QObject::connect(w, SIGNAL(signal3()), a, SLOT(slot3()), Qt::QueuedConnection);
    

    Change that to...

    signal_serialiser signal_serialiser;
    QObject::connect(w, SIGNAL(signal1()), &signal_serialiser, SIGNAL(signal1()));
    QObject::connect(w, SIGNAL(signal2()), &signal_serialiser, SIGNAL(signal2()));
    QObject::connect(w, SIGNAL(signal3()), &signal_serialiser, SIGNAL(signal3()));
    
    /*
     * Note the use of Qt::BlockingQueuedConnection for the
     * signal_serialiser --> A connections.
     */
    QObject::connect(&signal_serialiser, SIGNAL(signal1()), a, SLOT(slot1()), Qt::BlockingQueuedConnection);
    QObject::connect(&signal_serialiser, SIGNAL(signal2()), a, SLOT(slot2()), Qt::BlockingQueuedConnection);
    QObject::connect(&signal_serialiser, SIGNAL(signal3()), a, SLOT(slot3()), Qt::BlockingQueuedConnection);
    QThread signal_serialiser_thread;
    signal_serialiser.moveToThread(&signal_serialiser_thread);
    signal_serialiser_thread.start();
    

    I've only done basic testing but it appears to give the desired behaviour.