Fix a leak and a crash in designer

We have never cleared the property sheets
extension objects for object on the form.
This was done on designer / creator exit,
but in meantime we always cumulated
all of them in memory, event when the
original widget or its form got deleted.
They were cleared since they were the children
of QDesignerAbstractPropertySheetFactory
which got deleted on app exit.

Since property sheets were not removed,
they were existing without their
original object. We have remembered pointers
to them in order to reload resources when needed.
That caused a crash since we have referred to
non-existing originals during resource reload.

We have leaked so many billions of sh(ee)ts
during last 10 years at least...

Task-number: QTCREATORBUG-17150
Change-Id: Icf655cccf93a680b874cec8b1be333ce718db062
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@qt.io>

3 files changed, 67 insertions, 8 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 <QtCore/qdebug.h>
 #include <QtCore/QList>
+#include <QtCore/QSet>
 #include <QtCore/QTimer>
 #include <QtWidgets/QMenu>
 #include <QtWidgets/QListWidget>
@@ -112,6 +113,15 @@ FormWindowBase::FormWindowBase(QDesignerFormEditorInterface *core, QWidget *pare
 
 FormWindowBase::~FormWindowBase()
 {
+    QSet<QDesignerPropertySheet *> 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<QTreeWidget *>(object) ||
             qobject_cast<QTableWidget *>(object) ||
             qobject_cast<QListWidget *>(object) ||
-            qobject_cast<QComboBox *>(object))
+            qobject_cast<QComboBox *>(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<QDesignerPropertySheet *>(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 8c4f0eef3..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;
 }
 
@@ -1672,8 +1670,14 @@ void QDesignerAbstractPropertySheetFactory::objectDestroyed(QObject *object)
     QMutableMapIterator<QObject*, QObject*> it(m_impl->m_extensions);
     while (it.hasNext()) {
         it.next();
-        if (it.key() == object || it.value() == object)
+        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();
+        }
     }
 }