From 8575ed82ff0dd43aed8f5de67b4cfef06fec8c33 Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Fri, 21 Oct 2016 15:08:05 +0200 Subject: Fix a leak and a crash in designer This change downports the 56ac0c12ab5e7269e16bcc1248e4241302d91d09 change to the 5.7.1 branch in order to have a chance for releasing Qt Creator 4.2 with a fix. The crash doesn't appear in the standalone Designer, it only appears in the Designer plugin of Qt Creator during reload. There is no other way to fix that crash outside of Qt. Task-number: QTCREATORBUG-17150 Change-Id: Ide521677a1526d3de862d0d8e283531a67b2a7a8 Reviewed-by: Friedemann Kleint (cherry picked from commit 56ac0c12ab5e7269e16bcc1248e4241302d91d09) --- src/designer/src/lib/shared/formwindowbase.cpp | 61 ++++++++++++++++++++-- src/designer/src/lib/shared/formwindowbase_p.h | 4 +- .../src/lib/shared/qdesigner_propertysheet.cpp | 35 ++++--------- 3 files changed, 71 insertions(+), 29 deletions(-) diff --git a/src/designer/src/lib/shared/formwindowbase.cpp b/src/designer/src/lib/shared/formwindowbase.cpp index e702d777a..d8f422a0a 100644 --- a/src/designer/src/lib/shared/formwindowbase.cpp +++ b/src/designer/src/lib/shared/formwindowbase.cpp @@ -47,6 +47,7 @@ #include #include +#include #include #include #include @@ -112,6 +113,15 @@ FormWindowBase::FormWindowBase(QDesignerFormEditorInterface *core, QWidget *pare FormWindowBase::~FormWindowBase() { + QSet sheets = m_d->m_reloadableResources.keys().toSet(); + sheets |= m_d->m_reloadablePropertySheets.keys().toSet(); + + m_d->m_reloadableResources.clear(); + m_d->m_reloadablePropertySheets.clear(); + + for (QDesignerPropertySheet *sheet : sheets) + disconnectSheet(sheet); + delete m_d; } @@ -137,14 +147,17 @@ void FormWindowBase::setResourceSet(QtResourceSet *resourceSet) void FormWindowBase::addReloadableProperty(QDesignerPropertySheet *sheet, int index) { + connectSheet(sheet); m_d->m_reloadableResources[sheet][index] = true; } void FormWindowBase::removeReloadableProperty(QDesignerPropertySheet *sheet, int index) { m_d->m_reloadableResources[sheet].remove(index); - if (m_d->m_reloadableResources[sheet].count() == 0) + if (!m_d->m_reloadableResources[sheet].count()) { m_d->m_reloadableResources.remove(sheet); + disconnectSheet(sheet); + } } void FormWindowBase::addReloadablePropertySheet(QDesignerPropertySheet *sheet, QObject *object) @@ -152,13 +165,53 @@ void FormWindowBase::addReloadablePropertySheet(QDesignerPropertySheet *sheet, Q if (qobject_cast(object) || qobject_cast(object) || qobject_cast(object) || - qobject_cast(object)) + qobject_cast(object)) { + connectSheet(sheet); m_d->m_reloadablePropertySheets[sheet] = object; + } } -void FormWindowBase::removeReloadablePropertySheet(QDesignerPropertySheet *sheet) +void FormWindowBase::connectSheet(QDesignerPropertySheet *sheet) { - m_d->m_reloadablePropertySheets.remove(sheet); + if (m_d->m_reloadableResources.contains(sheet) + || m_d->m_reloadablePropertySheets.contains(sheet)) { + // already connected + return; + } + connect(sheet, &QObject::destroyed, this, &FormWindowBase::sheetDestroyed); +} + +void FormWindowBase::disconnectSheet(QDesignerPropertySheet *sheet) +{ + if (m_d->m_reloadableResources.contains(sheet) + || m_d->m_reloadablePropertySheets.contains(sheet)) { + // still need to be connected + return; + } + disconnect(sheet, &QObject::destroyed, this, &FormWindowBase::sheetDestroyed); +} + +void FormWindowBase::sheetDestroyed(QObject *object) +{ + // qobject_cast(object) + // will fail since the destructor of QDesignerPropertySheet + // has already finished + + for (auto it = m_d->m_reloadableResources.begin(); + it != m_d->m_reloadableResources.end(); ++it) { + if (it.key() == object) { + m_d->m_reloadableResources.erase(it); + break; + } + } + + for (auto it = m_d->m_reloadablePropertySheets.begin(); + it != m_d->m_reloadablePropertySheets.end(); ++it) { + if (it.key() == object) { + m_d->m_reloadablePropertySheets.erase(it); + break; + } + } } void FormWindowBase::reloadProperties() diff --git a/src/designer/src/lib/shared/formwindowbase_p.h b/src/designer/src/lib/shared/formwindowbase_p.h index 0740e48a6..ceb8beb80 100644 --- a/src/designer/src/lib/shared/formwindowbase_p.h +++ b/src/designer/src/lib/shared/formwindowbase_p.h @@ -138,7 +138,6 @@ public: void addReloadableProperty(QDesignerPropertySheet *sheet, int index); void removeReloadableProperty(QDesignerPropertySheet *sheet, int index); void addReloadablePropertySheet(QDesignerPropertySheet *sheet, QObject *object); - void removeReloadablePropertySheet(QDesignerPropertySheet *sheet); void reloadProperties(); void emitWidgetRemoved(QWidget *w); @@ -167,9 +166,12 @@ public slots: private slots: void triggerDefaultAction(QWidget *w); + void sheetDestroyed(QObject *object); private: void syncGridFeature(); + void connectSheet(QDesignerPropertySheet *sheet); + void disconnectSheet(QDesignerPropertySheet *sheet); FormWindowBasePrivate *m_d; }; diff --git a/src/designer/src/lib/shared/qdesigner_propertysheet.cpp b/src/designer/src/lib/shared/qdesigner_propertysheet.cpp index 3caaba690..6a0910e2a 100644 --- a/src/designer/src/lib/shared/qdesigner_propertysheet.cpp +++ b/src/designer/src/lib/shared/qdesigner_propertysheet.cpp @@ -699,8 +699,6 @@ QDesignerPropertySheet::QDesignerPropertySheet(QObject *object, QObject *parent) QDesignerPropertySheet::~QDesignerPropertySheet() { - if (d->m_fwb) - d->m_fwb->removeReloadablePropertySheet(this); delete d; } @@ -1627,8 +1625,6 @@ struct QDesignerAbstractPropertySheetFactory::PropertySheetFactoryPrivate { typedef QMap ExtensionMap; ExtensionMap m_extensions; - typedef QHash ExtendedSet; - ExtendedSet m_extended; }; QDesignerAbstractPropertySheetFactory::PropertySheetFactoryPrivate::PropertySheetFactoryPrivate() : @@ -1653,30 +1649,20 @@ QDesignerAbstractPropertySheetFactory::~QDesignerAbstractPropertySheetFactory() QObject *QDesignerAbstractPropertySheetFactory::extension(QObject *object, const QString &iid) const { - typedef PropertySheetFactoryPrivate::ExtensionMap ExtensionMap; if (!object) return 0; if (iid != m_impl->m_propertySheetId && iid != m_impl->m_dynamicPropertySheetId) return 0; - ExtensionMap::iterator it = m_impl->m_extensions.find(object); - if (it == m_impl->m_extensions.end()) { - if (QObject *ext = createPropertySheet(object, const_cast(this))) { - connect(ext, &QObject::destroyed, this, &QDesignerAbstractPropertySheetFactory::objectDestroyed); - it = m_impl->m_extensions.insert(object, ext); - } - } - - if (!m_impl->m_extended.contains(object)) { + QObject *ext = m_impl->m_extensions.value(object, 0); + if (!ext && (ext = createPropertySheet(object, const_cast(this)))) { + connect(ext, &QObject::destroyed, this, &QDesignerAbstractPropertySheetFactory::objectDestroyed); connect(object, &QObject::destroyed, this, &QDesignerAbstractPropertySheetFactory::objectDestroyed); - m_impl->m_extended.insert(object, true); + m_impl->m_extensions.insert(object, ext); } - if (it == m_impl->m_extensions.end()) - return 0; - - return it.value(); + return ext; } void QDesignerAbstractPropertySheetFactory::objectDestroyed(QObject *object) @@ -1684,14 +1670,15 @@ void QDesignerAbstractPropertySheetFactory::objectDestroyed(QObject *object) QMutableMapIterator it(m_impl->m_extensions); while (it.hasNext()) { it.next(); - - QObject *o = it.key(); - if (o == object || object == it.value()) { + if (it.key() == object || it.value() == object) { + if (it.key() == object) { + QObject *ext = it.value(); + disconnect(ext, &QObject::destroyed, this, &QDesignerAbstractPropertySheetFactory::objectDestroyed); + delete ext; + } it.remove(); } } - - m_impl->m_extended.remove(object); } QT_END_NAMESPACE -- cgit v1.2.1