From b3b2bc3f190d80e30b968648dd91048df67b8a50 Mon Sep 17 00:00:00 2001 From: Thierry Bastian Date: Wed, 12 Aug 2009 10:33:33 +0200 Subject: Possible Dead lock in the destructor of QObject The problem was that we were locking a mutex that was global to thread to remove posted events associated with a QObject from the posted event list. We were also immediately deleting those events. If that triggers the deletion of another QObject, you would then trigger a dead-lock. Task-number: 259514 Reviewed-by: brad Reviewed-by: ogoffart --- src/corelib/kernel/qcoreapplication.cpp | 18 ++++++++++-------- src/corelib/kernel/qcoreapplication_p.h | 1 - src/corelib/kernel/qobject.cpp | 5 +---- tests/auto/qobject/tst_qobject.cpp | 18 ++++++++++++++++++ 4 files changed, 29 insertions(+), 13 deletions(-) diff --git a/src/corelib/kernel/qcoreapplication.cpp b/src/corelib/kernel/qcoreapplication.cpp index b90f5c7..86221a1 100644 --- a/src/corelib/kernel/qcoreapplication.cpp +++ b/src/corelib/kernel/qcoreapplication.cpp @@ -60,6 +60,7 @@ #include #include #include +#include #include #include @@ -1353,13 +1354,10 @@ void QCoreApplication::removePostedEvents(QObject *receiver, int eventType) // for this object. if (receiver && !receiver->d_func()->postedEvents) return; - QCoreApplicationPrivate::removePostedEvents_unlocked(receiver, eventType, data); -} -void QCoreApplicationPrivate::removePostedEvents_unlocked(QObject *receiver, - int eventType, - QThreadData *data) -{ + //we will collect all the posted events for the QObject + //and we'll delete after the mutex was unlocked + QVarLengthArray events; int n = data->postEventList.size(); int j = 0; @@ -1374,7 +1372,7 @@ void QCoreApplicationPrivate::removePostedEvents_unlocked(QObject *receiver, pe.receiver->d_func()->removePendingChildInsertedEvents(0); #endif pe.event->posted = false; - delete pe.event; + events.append(pe.event); const_cast(pe).event = 0; } else if (!data->postEventList.recursion) { if (i != j) @@ -1393,8 +1391,12 @@ void QCoreApplicationPrivate::removePostedEvents_unlocked(QObject *receiver, // truncate list data->postEventList.erase(data->postEventList.begin() + j, data->postEventList.end()); } -} + locker.unlock(); + for (int i = 0; i < events.count(); ++i) { + delete events[i]; + } +} /*! Removes \a event from the queue of posted events, and emits a diff --git a/src/corelib/kernel/qcoreapplication_p.h b/src/corelib/kernel/qcoreapplication_p.h index bb94cbb..ef776c7 100644 --- a/src/corelib/kernel/qcoreapplication_p.h +++ b/src/corelib/kernel/qcoreapplication_p.h @@ -90,7 +90,6 @@ public: static QThread *mainThread(); static bool checkInstance(const char *method); static void sendPostedEvents(QObject *receiver, int event_type, QThreadData *data); - static void removePostedEvents_unlocked(QObject *receiver, int type, QThreadData *data); #if !defined (QT_NO_DEBUG) || defined (QT_MAC_FRAMEWORK_BUILD) void checkReceiverThread(QObject *receiver); diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp index 3902825..2afab1a 100644 --- a/src/corelib/kernel/qobject.cpp +++ b/src/corelib/kernel/qobject.cpp @@ -896,10 +896,7 @@ QObject::~QObject() qt_removeObject(this); - QMutexLocker locker2(&d->threadData->postEventList.mutex); - if (d->postedEvents > 0) - QCoreApplicationPrivate::removePostedEvents_unlocked(this, 0, d->threadData); - locker2.unlock(); + QCoreApplication::removePostedEvents(this); if (d->parent) // remove it from parent object d->setParent_helper(0); diff --git a/tests/auto/qobject/tst_qobject.cpp b/tests/auto/qobject/tst_qobject.cpp index 2823c71..bb00a0b 100644 --- a/tests/auto/qobject/tst_qobject.cpp +++ b/tests/auto/qobject/tst_qobject.cpp @@ -119,6 +119,7 @@ private slots: void qobjectConstCast(); void uniqConnection(); void interfaceIid(); + void deleteQObjectWhenDeletingEvent(); protected: }; @@ -2898,5 +2899,22 @@ void tst_QObject::interfaceIid() QByteArray()); } +void tst_QObject::deleteQObjectWhenDeletingEvent() +{ + //this is related to task 259514 + //before the fix this used to dead lock when the QObject from the event was destroyed + + struct MyEvent : public QEvent + { + MyEvent() : QEvent(QEvent::User) { } + QObject obj; + }; + + QObject o; + QApplication::postEvent(&o, new MyEvent); + QCoreApplication::removePostedEvents(&o); // here you would get a deadlock +} + + QTEST_MAIN(tst_QObject) #include "tst_qobject.moc" -- cgit v0.12