Is it safe to terminate the thread that emits a queued signal before the signal has been processed?

Viewed 110

In my code I have a worker that runs in a separate std::jthread:

// Worker.h:
#pragma once
#include <QWidget>
class Worker : public QObject {
    Q_OBJECT
public:
    Worker();
signals:
    void sendMessage(QString msg);
private:
    void threadFunction();
    std::jthread m_thread;
};

and a widget that runs in the main thread:

// Widget.h
#pragma once
#include <QWidget>
class Widget : public QWidget {
    Q_OBJECT
public:
    Widget();
public slots:
    void getMessage(QString msg);
};

I create a queued connection between the sendMessage signal and getMessage slot:

#include <QApplication>
#include <chrono>
#include "Widget.h"
#include "Worker.h"

int main(int argc, char* argv[])
{
    QApplication a(argc, argv);
    Widget widget;
    {
        Worker worker;
        QObject::connect(&worker, &Worker::sendMessage, &widget, &Widget::getMessage, 
                         Qt::ConnectionType::QueuedConnection);
        std::this_thread::sleep_for(std::chrono::seconds(1));
    } // worker deleted and thread terminated
    widget.show();
    return a.exec();
}

Q1: Is it safe to terminate the thread that emits the signal before the signal has been processed?

 // Worker.cpp:
 #include "Worker.h"
 #include <chrono>
 Worker::Worker() {
    m_thread = std::jthread(&Worker::threadFunction, this);
 }
 void Worker::threadFunction() {
    std::this_thread::sleep_for(std::chrono::seconds(1));
    emit sendMessage("finished");
 }

and

// Widget.cpp:
#include "Widget.h"
#include <iostream>

Widget::Widget() : QWidget() {}

void Widget::getMessage(QString msg) {
    std::cerr << msg.toStdString() << "\n";
}

The emit of sendMessage invokes the method:

// moc_Worker.cpp:
void Worker::sendMessage(QString _t1)
{
    void *_a[] = { nullptr, const_cast<void*>(reinterpret_cast<const void*>(std::addressof(_t1))) };
    QMetaObject::activate(this, &staticMetaObject, 0, _a);
}

QMetaObject::activate() is defined in qobject.cpp. Reading about how Qt signals and slots are implemented I find that this method looks up the zeroth element (first and only signal) of a SignalVector of the Worker object. This element is a doubly linked list of QObjectPrivate::Connection objects. In my case this list only have 1 element which is a connection to the getMessage method.

It also calls

QThreadData *td = connection->receiverThreadData.loadRelaxed();
bool receiverInSameThread = currentThreadId == td->threadId.loadRelaxed();

on this connection to determine if the receiver is in the same thread as the sender. The receiverThreadData is being set when the connection is made:

// qobject.cpp:
QObjectPrivate::Connection *QMetaObjectPrivate::connect(...,QObject *receiver,...) {
   ...
   QThreadData *td = receiver->threadData;
   connection->receiverThreadData.storeRelaxed(td);
   ...
}

Since I am using a QueuedConnection the method:

void QObject::queued_activate(QObject *sender, int signal, QObjectPrivate::Connection *c, void **argv);

is called which in turn calls:

void QCoreApplication::postEvent(QObject *receiver, QEvent *event);

with a QMetaCallEvent:

struct QMetaCallEvent {
   // the signal:
   QObject *sender; 
   uint signal_index;
     
   // the slot:
   QObject *receiver;
   ushort method_offset;
       
   // the data:
   int nargs_;
   int* types_;
   void** args_;
};

Q2: Is it safe to delete the sender object before the event has been processed?

Reading about QObject *QObject::sender(): "Returns a pointer to the object that sent the signal, if called in a slot activated by a signal; otherwise it returns nullptr. The pointer is valid only during the execution of the slot that calls this function from this object's thread context.

The pointer returned by this function becomes invalid if the sender is destroyed, or if the slot is disconnected from the sender's signal."

Invalid pointer = may crash?

1 Answers

It should be safe to process a signal after the emitting thread is terminated and/or the sending object is destroyed.

Queued connections are meant to provide easy, non-blocking and thread-safe communication between threads. Although not explicitly documented, this mechanism would become useless if it is not safe to shut down a thread before all emitted signals are processed in another thread, as the emitting thread/object is not aware of when the slot is processed.

Off course,

  • You should ensure that the passed objects are not destructed (in case of (smart) pointer types are passed).
  • You do not access any object that was destructed in the other thread (i.e. QObject::sender()).
Related