diff options
| author | Thiago Macieira <thiago.macieira@intel.com> | 2023-03-21 21:38:11 -1000 |
|---|---|---|
| committer | Thiago Macieira <thiago.macieira@intel.com> | 2023-03-23 13:22:23 -1000 |
| commit | 4d90c4e74a6aa34d5aabfb91ec304da1f02df3e3 (patch) | |
| tree | f0b0e786e7157d5677b10f909da7074a1f173696 /src/corelib/kernel/qtimer.cpp | |
| parent | f7167ae82fd74d69f2b138fb60a36ed7f863b380 (diff) | |
QTimer: fix new-style slot invocation for receiver in another thread
For single-shot timers, at least. QSingleShotTimer had either an
optimization or the only way to make new-style slot invocations by
storing the QSlotObject pointer and calling it directly. Instead of
doing that, let's just actually connect and let QObject handle the
actual delivery.
[ChangeLog][QtCore][QTimer] Fixed a bug that caused slots connected to
single-slot timers using the new-style connection mechanism to be
delivered in the wrong thread.
Fixes: QTBUG-112162
Pick-to: 5.15 6.2 6.5
Change-Id: Idd5e1bb52be047d7b4fffffd174eadb227ab65ee
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
Diffstat (limited to 'src/corelib/kernel/qtimer.cpp')
| -rw-r--r-- | src/corelib/kernel/qtimer.cpp | 33 |
1 files changed, 13 insertions, 20 deletions
diff --git a/src/corelib/kernel/qtimer.cpp b/src/corelib/kernel/qtimer.cpp index 73f0fb8fee3..9c074a064f8 100644 --- a/src/corelib/kernel/qtimer.cpp +++ b/src/corelib/kernel/qtimer.cpp @@ -7,10 +7,11 @@ #include "qabstracteventdispatcher.h" #include "qcoreapplication.h" -#include "qobject_p.h" -#include "qthread.h" #include "qcoreapplication_p.h" +#include "qmetaobject_p.h" +#include "qobject_p.h" #include "qproperty_p.h" +#include "qthread.h" QT_BEGIN_NAMESPACE @@ -249,9 +250,6 @@ class QSingleShotTimer : public QObject { Q_OBJECT int timerId; - bool hasValidReceiver; - QPointer<const QObject> receiver; - QtPrivate::QSlotObjectBase *slotObj; public: ~QSingleShotTimer(); QSingleShotTimer(int msec, Qt::TimerType timerType, const QObject *r, const char * m); @@ -264,16 +262,23 @@ protected: }; QSingleShotTimer::QSingleShotTimer(int msec, Qt::TimerType timerType, const QObject *r, const char *member) - : QObject(QAbstractEventDispatcher::instance()), hasValidReceiver(true), slotObj(nullptr) + : QObject(QAbstractEventDispatcher::instance()) { timerId = startTimer(std::chrono::milliseconds{msec}, timerType); connect(this, SIGNAL(timeout()), r, member); } QSingleShotTimer::QSingleShotTimer(int msec, Qt::TimerType timerType, const QObject *r, QtPrivate::QSlotObjectBase *slotObj) - : QObject(QAbstractEventDispatcher::instance()), hasValidReceiver(r), receiver(r), slotObj(slotObj) + : QObject(QAbstractEventDispatcher::instance()) { timerId = startTimer(std::chrono::milliseconds{msec}, timerType); + + int signal_index = QMetaObjectPrivate::signalOffset(&staticMetaObject); + Q_ASSERT(QMetaObjectPrivate::signal(&staticMetaObject, signal_index).name() == "timeout"); + QObjectPrivate::connectImpl(this, signal_index, r ? r : this, nullptr, slotObj, + Qt::AutoConnection, nullptr, &staticMetaObject); + + // ### Why is this here? Why doesn't the case above need it? if (r && thread() != r->thread()) { // Avoid leaking the QSingleShotTimer instance in case the application exits before the timer fires connect(QCoreApplication::instance(), &QCoreApplication::aboutToQuit, this, &QObject::deleteLater); @@ -286,8 +291,6 @@ QSingleShotTimer::~QSingleShotTimer() { if (timerId > 0) killTimer(timerId); - if (slotObj) - slotObj->destroyIfLastRef(); } void QSingleShotTimer::timerEvent(QTimerEvent *) @@ -298,17 +301,7 @@ void QSingleShotTimer::timerEvent(QTimerEvent *) killTimer(timerId); timerId = -1; - if (slotObj) { - // If the receiver was destroyed, skip this part - if (Q_LIKELY(!receiver.isNull() || !hasValidReceiver)) { - // We allocate only the return type - we previously checked the function had - // no arguments. - void *args[1] = { nullptr }; - slotObj->call(const_cast<QObject*>(receiver.data()), args); - } - } else { - emit timeout(); - } + emit timeout(); // we would like to use delete later here, but it feels like a // waste to post a new event to handle this event, so we just unset the flag |