From 0c643b179c5154c50b61dba421016b7b48794720 Mon Sep 17 00:00:00 2001 From: "Bradley T. Hughes" Date: Fri, 29 Oct 2010 15:04:46 +0200 Subject: Simplify object lifetime management when moving objects to a QThread The documentation for Qt::AutoConnection states is a signal is emitted from the same thread as the receiving object, the connection should behave as Qt::DirectConnection. The actual behavior prior to this commit was different from the documentation; if the signal was emitted in a thread different from the sender's thread, then we would queue (which is now corrected). By making the behavior match the documentation, it is now possible to connect QThread's finished() signal to an object's deleteLater() slot and the slot will execute when finished() is emitted (previously it was queued). QObject::deleteLater() uses a posted QEvent::DeferredDelete event to trigger deletion of the object. In QThread, after emitting the finished() signal, we now send all pending DeferredDelete events to ensure that all pending deletions happen. We have precedence for this behavior, QCoreApplication::exec() does the same thing after emitting the aboutToQuit() signal. Reviewed-by: joao Reviewed-by: olivier --- src/corelib/kernel/qobject.cpp | 4 +- src/corelib/thread/qthread_unix.cpp | 1 + src/corelib/thread/qthread_win.cpp | 1 + tests/auto/qobject/tst_qobject.cpp | 78 +++++++++++++++++++++++++++++++++++++ tests/auto/qthread/tst_qthread.cpp | 14 +++++++ 5 files changed, 95 insertions(+), 3 deletions(-) diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp index 573bb50..7fe9c52 100644 --- a/src/corelib/kernel/qobject.cpp +++ b/src/corelib/kernel/qobject.cpp @@ -3508,9 +3508,7 @@ void QMetaObject::activate(QObject *sender, const QMetaObject *m, int local_sign // determine if this connection should be sent immediately or // put into the event queue - if ((c->connectionType == Qt::AutoConnection - && (!receiverInSameThread - || receiver->d_func()->threadData != sender->d_func()->threadData)) + if ((c->connectionType == Qt::AutoConnection && !receiverInSameThread) || (c->connectionType == Qt::QueuedConnection)) { queued_activate(sender, signal_absolute_index, c, argv ? argv : empty_argv); continue; diff --git a/src/corelib/thread/qthread_unix.cpp b/src/corelib/thread/qthread_unix.cpp index a44a0e8..e39e577 100644 --- a/src/corelib/thread/qthread_unix.cpp +++ b/src/corelib/thread/qthread_unix.cpp @@ -343,6 +343,7 @@ void QThreadPrivate::finish(void *arg) emit thr->terminated(); d->terminated = false; emit thr->finished(); + QCoreApplication::sendPostedEvents(0, QEvent::DeferredDelete); if (d->data->eventDispatcher) { d->data->eventDispatcher->closingDown(); diff --git a/src/corelib/thread/qthread_win.cpp b/src/corelib/thread/qthread_win.cpp index f0cbe8d..4a967ed 100644 --- a/src/corelib/thread/qthread_win.cpp +++ b/src/corelib/thread/qthread_win.cpp @@ -332,6 +332,7 @@ void QThreadPrivate::finish(void *arg, bool lockAnyway) emit thr->terminated(); d->terminated = false; emit thr->finished(); + QCoreApplication::sendPostedEvents(0, QEvent::DeferredDelete); if (d->data->eventDispatcher) { d->data->eventDispatcher->closingDown(); diff --git a/tests/auto/qobject/tst_qobject.cpp b/tests/auto/qobject/tst_qobject.cpp index 24cd5a3..a09f109 100644 --- a/tests/auto/qobject/tst_qobject.cpp +++ b/tests/auto/qobject/tst_qobject.cpp @@ -134,6 +134,7 @@ private slots: void connectConstructorByMetaMethod(); void disconnectByMetaMethod(); void disconnectNotSignalMetaMethod(); + void autoConnectionBehavior(); protected: }; @@ -3847,5 +3848,82 @@ void tst_QObject::disconnectNotSignalMetaMethod() QVERIFY(!QObject::disconnect(&s, slot, &r, QMetaMethod())); } +class ThreadAffinityThread : public QThread +{ +public: + SenderObject *sender; + + ThreadAffinityThread(SenderObject *sender) + : sender(sender) + { } + void run() + { + sender->emitSignal1(); + } +}; + +void tst_QObject::autoConnectionBehavior() +{ + SenderObject *sender = new SenderObject; + ReceiverObject *receiver = new ReceiverObject; + connect(sender, SIGNAL(signal1()), receiver, SLOT(slot1())); + + // at emit, currentThread == sender->thread(), currentThread == receiver->thread(), sender->thread() == receiver->thread() + QVERIFY(!receiver->called(1)); + sender->emitSignal1(); + QVERIFY(receiver->called(1)); + receiver->reset(); + + // at emit, currentThread != sender->thread(), currentThread != receiver->thread(), sender->thread() == receiver->thread() + ThreadAffinityThread emitThread1(sender); + QVERIFY(!receiver->called(1)); + emitThread1.start(); + QVERIFY(emitThread1.wait(30000)); + QVERIFY(!receiver->called(1)); + QCoreApplication::sendPostedEvents(receiver, QEvent::MetaCall); + QVERIFY(receiver->called(1)); + receiver->reset(); + + // at emit, currentThread == sender->thread(), currentThread != receiver->thread(), sender->thread() != receiver->thread() + sender->moveToThread(&emitThread1); + QVERIFY(!receiver->called(1)); + emitThread1.start(); + QVERIFY(emitThread1.wait(30000)); + QVERIFY(!receiver->called(1)); + QCoreApplication::sendPostedEvents(receiver, QEvent::MetaCall); + QVERIFY(receiver->called(1)); + receiver->reset(); + + // at emit, currentThread != sender->thread(), currentThread == receiver->thread(), sender->thread() != receiver->thread() + QVERIFY(!receiver->called(1)); + sender->emitSignal1(); + QVERIFY(receiver->called(1)); + receiver->reset(); + + // at emit, currentThread != sender->thread(), currentThread != receiver->thread(), sender->thread() != receiver->thread() + ThreadAffinityThread emitThread2(sender); + QThread receiverThread; + QTimer *timer = new QTimer; + timer->setSingleShot(true); + timer->setInterval(100); + connect(&receiverThread, SIGNAL(started()), timer, SLOT(start())); + connect(timer, SIGNAL(timeout()), &receiverThread, SLOT(quit()), Qt::DirectConnection); + connect(&receiverThread, SIGNAL(finished()), timer, SLOT(deleteLater())); + timer->moveToThread(&receiverThread); + + receiver->moveToThread(&receiverThread); + QVERIFY(!receiver->called(1)); + emitThread2.start(); + QVERIFY(emitThread2.wait(30000)); + QVERIFY(!receiver->called(1)); + receiverThread.start(); + QVERIFY(receiverThread.wait(30000)); + QVERIFY(receiver->called(1)); + receiver->reset(); + + delete sender; + delete receiver; +} + QTEST_MAIN(tst_QObject) #include "tst_qobject.moc" diff --git a/tests/auto/qthread/tst_qthread.cpp b/tests/auto/qthread/tst_qthread.cpp index 843749a..f290a2b 100644 --- a/tests/auto/qthread/tst_qthread.cpp +++ b/tests/auto/qthread/tst_qthread.cpp @@ -106,6 +106,7 @@ private slots: void adoptMultipleThreads(); void QTBUG13810_exitAndStart(); + void connectThreadFinishedSignalToObjectDeleteLaterSlot(); void stressTest(); }; @@ -975,6 +976,19 @@ void tst_QThread::QTBUG13810_exitAndStart() QCOMPARE(sync1.m_prop, 89); } +void tst_QThread::connectThreadFinishedSignalToObjectDeleteLaterSlot() +{ + QThread thread; + QObject *object = new QObject; + QWeakPointer p = object; + QVERIFY(!p.isNull()); + connect(&thread, SIGNAL(started()), &thread, SLOT(quit()), Qt::DirectConnection); + connect(&thread, SIGNAL(finished()), object, SLOT(deleteLater())); + object->moveToThread(&thread); + thread.start(); + QVERIFY(thread.wait(30000)); + QVERIFY(p.isNull()); +} QTEST_MAIN(tst_QThread) #include "tst_qthread.moc" -- cgit v0.12