Qt::BlockingQueuedConnection not blocking, according to thread sanitizer

bobthebuilder

Hi Qt folks!

I have an application with two threads. The main thread handles UI type tasks, while a worker thread has an event loop and does a very slow background process when signaled by the main thread. When the worker thread is done with the slow background process it signals the main thread to copy over the data. I connected the worker's completion signal to the main object's copy data slot with a Qt::BlockingQueuedConnection to ensure the data is copied before the worker thread starts processing another event and thus changes the data, which would create a race. I was under the impression that Qt::BlockingQueuedConnection in this case would cause the worker to block all execution until the main object's slot returns, but this doesn't seem to be happening. If there are multiple slow background process events in the worker's queue, it seems to ignore the Qt::BlockingQueuedConnection and immediately starts processing another slow background event, at least this is what the LLVM thread sanitizer is indicating.

Here is a sketch of my code:

// ... in the main object that contains the worker object and thread
QThread worker_thread;
WorkerObject* worker_qobject = new WorkerObject;
connect(worker_qobject, &WorkerObject::dataCreated, this, &MainObject::copyData, Qt::BlockingQueuedConnection);
worker_qobject->moveToThread(&worker_thread);
worker_thread.start();

// Slot in the main object to copy data from the worker
MainObject::copyData()
{
    main_data = worker_qobject.data;
}

// Method/slot in the worker that does slow data creation and signals the main thread
WorkerObject::createData()
{
    slowDataCreation(); // Modifies the data member
    emit dataCreated();
}

If multiple createData calls are in the worker thread's queue, it doesn't seem to block until copyData returns, but instead immediately begins execution of createData, thus leading to a race. Does anyone have an idea what I'm doing wrong here?

jeremy_k

It would help to have a minimal, complete example. The code snippet above leaves lots of questions, such as what WorkerObject::data is, and what the corresponding operator=() does. Rather than play whack-a-mole by asking about individual points, it is much easier to examine the entire problem.

bobthebuilder

Excellent suggestion, here is a hopefully complete yet minimal version of my setup:

class WorkerObject final : public QObject
{

  Q_OBJECT

public:

  std::vector<double> m_processed_data;

  void expensiveDataProcessing()
  {
    // Long running operation to populate large vector m_processed_data
  }

public slots:

  void processData()
  {
    expensiveDataProcessing();
    emit dataCreated();
  }

signals:

  void dataCreated();

};

class MainWidget final : public QWidget
{

  Q_OBJECT

public:

  QThread m_worker_thread;
  WorkerObject* m_worker_object;
  std::vector<double> m_main_data_copy;

  MainWidget()
  {
    m_worker_object = new WorkerObject;
    m_worker_object->moveToThread(&m_worker_thread);

    // Some action can get triggered indicating the user wants more data processed
    connect(some_user_triggered_action, &QAction::triggered, [this](){emit processData();});

    connect(this, &MainWidget::processData, m_worker_object, &WorkerObject::processData);
    connect(m_worker_object, &WorkerObject::dataCreated, this, &MainWidget::copyData, Qt::BlockingQueuedConnection);

    m_worker_thread.start();
  }

public slots:

  void copyData()
  {
    m_main_data_copy = m_worker_object->m_processed_data;
  }

signals:

  void processData();

};

When the worker has multiple processData calls queued up, it doesn't seem to block after signaling main's copyData, triggering a data race.

bobthebuilder

Is there a chance this is a false positive with LLVM's thread sanitizer? I'm trying to artificially generate a race by adding a huge artificial delay into the MainWidget's copyData call and intentionally queuing up multiple processData calls at once for the WorkerObject, with some plain old printf debugging statements bracketing each function, and everything is synchronized as expected. With Qt::BlockingQueuedConnection, processData is never called again until copyData returns. If I remove Qt::BlockingQueuedConnection then I can trigger a race...

jeremy_k

@bobthebuilder said in Qt::BlockingQueuedConnection not blocking, according to thread sanitizer:

Is there a chance this is a false positive with LLVM's thread sanitizer?

That's possible. I created a complete* example from the second snippet above, which behaves correctly in ad hoc testing. It would be interested to know if tsan also complains about it.

#include <QCoreApplication>
#include <QTimer>
#include <QThread>
#include <QDebug>
#include <chrono>

auto copySleep = std::chrono::milliseconds(100);
auto genSleep = std::chrono::milliseconds(1);

class WorkerObject: public QObject
{
    Q_OBJECT

public slots:
    void processData()
    {
        static unsigned int index = 0;
        qDebug() << "create start  " << ++index;
        emit dataCreated(index);
        qDebug() << "create end    " << index;
    }

signals:
    void dataCreated(unsigned int index);
};

class MainWidget: public QObject
{
    Q_OBJECT

public:
    QThread m_worker_thread;
    WorkerObject m_worker_object;
    QTimer t;

    MainWidget()
    {
        m_worker_object.moveToThread(&m_worker_thread);

        t.setInterval(genSleep);
        t.start();

        t.callOnTimeout(&m_worker_object, &WorkerObject::processData);
        QObject::connect(&m_worker_object, &WorkerObject::dataCreated, this, &MainWidget::copyData, Qt::BlockingQueuedConnection);

        m_worker_thread.start();
    }

public slots:
    void copyData(unsigned int index)
    {
        qDebug() << "    copy start" << index;
        QThread::sleep(copySleep);
        qDebug() << "    copy end  " << index;
    }
};

int main(int argc, char *argv[])
{
    QCoreApplication a(argc, argv);
    MainWidget mw;
    return a.exec();
}

#include "main.moc"

* Minus standard Qt development environment and project file

Christian Ehrlicher

@bobthebuilder said in Qt::BlockingQueuedConnection not blocking, according to thread sanitizer:

void copyData()
{
m_main_data_copy = m_worker_object->m_processed_data;
}

The thread sanitizer is correct here - you access data from one thread in another without proper locking. It does not know (how should it) that the other thread is sleeping during this operation.

From my pov this approach is faulty by design - don't use a blocking connection but store the data e.g. in std::shared_ptr<std::vector<double>>, pass this to your signal and work on a new instance. So no blocking is needed at all and the sanitizer won't blame and it also will work without some Qt magic.