From 43619db05d55ca619dac11fdb7327b2b45507cb9 Mon Sep 17 00:00:00 2001 From: Lars Knoll Date: Tue, 20 Nov 2012 13:34:06 +0100 Subject: [PATCH] Emit destroyed() signal before children get deleted Make sure we always emit the destroyed() signal before we delete our children. This wasn't working correctly for QWidget based classes, as the QWidget destructor deletes all children itself. Task-number: QTBUG-24672 Change-Id: Iecdff3489196271177edfeba1c4a2c5800e255af Reviewed-by: Olivier Goffart --- src/corelib/kernel/qobject.cpp | 2 +- src/widgets/kernel/qwidget.cpp | 18 ++++ .../widgets/kernel/qwidget/tst_qwidget.cpp | 100 ++++++++++++++++++ 3 files changed, 119 insertions(+), 1 deletion(-) diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp index 0d97266f4f7..9d0854f3a9d 100644 --- a/src/corelib/kernel/qobject.cpp +++ b/src/corelib/kernel/qobject.cpp @@ -766,7 +766,7 @@ QObject::~QObject() delete sharedRefcount; } - if (d->isSignalConnected(0)) { + if (!d->isWidget && d->isSignalConnected(0)) { QT_TRY { emit destroyed(this); } QT_CATCH(...) { diff --git a/src/widgets/kernel/qwidget.cpp b/src/widgets/kernel/qwidget.cpp index 83dc406e179..d05f53697ae 100644 --- a/src/widgets/kernel/qwidget.cpp +++ b/src/widgets/kernel/qwidget.cpp @@ -1429,11 +1429,29 @@ QWidget::~QWidget() delete d->needsFlush; d->needsFlush = 0; + // The next 20 lines are duplicated from QObject, but required here + // since QWidget deletes is children itself + bool blocked = d->blockSig; + d->blockSig = 0; // unblock signals so we always emit destroyed() + + if (d->isSignalConnected(0)) { + QT_TRY { + emit destroyed(this); + } QT_CATCH(...) { + // all the signal/slots connections are still in place - if we don't + // quit now, we will crash pretty soon. + qWarning("Detected an unexpected exception in ~QWidget while emitting destroyed()."); + QT_RETHROW; + } + } + if (d->declarativeData) { QAbstractDeclarativeData::destroyed(d->declarativeData, this); d->declarativeData = 0; // don't activate again in ~QObject } + d->blockSig = blocked; + #ifdef Q_WS_MAC // QCocoaView holds a pointer back to this widget. Clear it now // to make sure it's not followed later on. The lifetime of the diff --git a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp index f3f63b20678..2a5e59f8252 100644 --- a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp +++ b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp @@ -396,6 +396,8 @@ private slots: void touchEventSynthesizedMouseEvent(); void styleSheetPropagation(); + + void destroyedSignal(); private: bool ensureScreenSize(int width, int height); QWidget *testWidget; @@ -9592,6 +9594,104 @@ void tst_QWidget::styleSheetPropagation() } } +class DestroyTester : public QObject +{ + Q_OBJECT +public: + DestroyTester(QObject *parent) : QObject(parent) { parentDestroyed = 0; } + static int parentDestroyed; +public slots: + void parentDestroyedSlot() { + ++parentDestroyed; + } +}; + +int DestroyTester::parentDestroyed = 0; + +void tst_QWidget::destroyedSignal() +{ + { + QWidget *w = new QWidget; + DestroyTester *t = new DestroyTester(w); + connect(w, SIGNAL(destroyed()), t, SLOT(parentDestroyedSlot())); + QCOMPARE(DestroyTester::parentDestroyed, 0); + delete w; + QCOMPARE(DestroyTester::parentDestroyed, 1); + } + + { + QWidget *w = new QWidget; + DestroyTester *t = new DestroyTester(w); + connect(w, SIGNAL(destroyed()), t, SLOT(parentDestroyedSlot())); + w->blockSignals(true); + QCOMPARE(DestroyTester::parentDestroyed, 0); + delete w; + QCOMPARE(DestroyTester::parentDestroyed, 1); + } + + { + QObject *o = new QWidget; + DestroyTester *t = new DestroyTester(o); + connect(o, SIGNAL(destroyed()), t, SLOT(parentDestroyedSlot())); + QCOMPARE(DestroyTester::parentDestroyed, 0); + delete o; + QCOMPARE(DestroyTester::parentDestroyed, 1); + } + + { + QObject *o = new QWidget; + DestroyTester *t = new DestroyTester(o); + connect(o, SIGNAL(destroyed()), t, SLOT(parentDestroyedSlot())); + o->blockSignals(true); + QCOMPARE(DestroyTester::parentDestroyed, 0); + delete o; + QCOMPARE(DestroyTester::parentDestroyed, 1); + } + + { + QWidget *w = new QWidget; + DestroyTester *t = new DestroyTester(0); + connect(w, SIGNAL(destroyed()), t, SLOT(parentDestroyedSlot())); + QCOMPARE(DestroyTester::parentDestroyed, 0); + delete w; + QCOMPARE(DestroyTester::parentDestroyed, 1); + delete t; + } + + { + QWidget *w = new QWidget; + DestroyTester *t = new DestroyTester(0); + connect(w, SIGNAL(destroyed()), t, SLOT(parentDestroyedSlot())); + w->blockSignals(true); + QCOMPARE(DestroyTester::parentDestroyed, 0); + delete w; + QCOMPARE(DestroyTester::parentDestroyed, 1); + delete t; + } + + { + QObject *o = new QWidget; + DestroyTester *t = new DestroyTester(0); + connect(o, SIGNAL(destroyed()), t, SLOT(parentDestroyedSlot())); + QCOMPARE(DestroyTester::parentDestroyed, 0); + delete o; + QCOMPARE(DestroyTester::parentDestroyed, 1); + delete t; + } + + { + QObject *o = new QWidget; + DestroyTester *t = new DestroyTester(0); + connect(o, SIGNAL(destroyed()), t, SLOT(parentDestroyedSlot())); + o->blockSignals(true); + QCOMPARE(DestroyTester::parentDestroyed, 0); + delete o; + QCOMPARE(DestroyTester::parentDestroyed, 1); + delete t; + } + +} + #ifndef QTEST_NO_CURSOR void tst_QWidget::underMouse() {