Don't leak memory when canceling Type Hierarchy

Add QFutureSynchronizer for collecting all running
futures (including those already canceled, but not finished yet).
It could happen, that we cancel the future, while
the associated task still needs some time in order
to catch the cancel request. When a d'tor of synchronizer
is run it cancels and waits for all pending futures
to be finished.

Add extra check for isCanceled() after a call to
updateDependencyTable(). In case the cancel was detected
inside updateDependencyTable(), we should return immediately.

Don't leak CppClass object inside handleLookupItemMatch()
when the task was canceled.

Amends: c400923308

Change-Id: I709e14a0c8b563d522a8e8c32b087e5f83310b24
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

src/plugins/cppeditor/cpptypehierarchy.cpp

@@ -130,15 +130,21 @@ CppTypeHierarchyWidget::CppTypeHierarchyWidget()
 
     connect(CppEditorPlugin::instance(), &CppEditorPlugin::typeHierarchyRequested,
             this, &CppTypeHierarchyWidget::perform);
-    connect(&m_futureWatcher, &QFutureWatcher<QSharedPointer<CppElement>>::finished,
+    connect(&m_futureWatcher, &QFutureWatcher<void>::finished,
             this, &CppTypeHierarchyWidget::displayHierarchy);
+
+    m_synchronizer.setCancelOnWait(true);
 }
 
-CppTypeHierarchyWidget::~CppTypeHierarchyWidget()
+void CppTypeHierarchyWidget::updateSynchronizer()
 {
-    if (m_future.isRunning()) {
-        m_future.cancel();
-        m_future.waitForFinished();
+    const QList<QFuture<void>> futures = m_synchronizer.futures();
+
+    m_synchronizer.clearFutures();
+
+    for (const QFuture<void> &future : futures) {
+        if (!future.isFinished())
+            m_synchronizer.addFuture(future);
     }
 }
 
@@ -147,6 +153,8 @@ void CppTypeHierarchyWidget::perform()
     if (m_future.isRunning())
         m_future.cancel();
 
+    updateSynchronizer();
+
     auto editor = qobject_cast<CppEditor *>(Core::EditorManager::currentEditor());
     if (!editor) {
         showNoTypeHierarchyLabel();
@@ -165,13 +173,15 @@ void CppTypeHierarchyWidget::perform()
     evaluator.setLookupBaseClasses(true);
     evaluator.setLookupDerivedClasses(true);
     m_future = evaluator.asyncExecute();
-    m_futureWatcher.setFuture(m_future);
+    m_futureWatcher.setFuture(QFuture<void>(m_future));
+    m_synchronizer.addFuture(QFuture<void>(m_future));
 
     Core::ProgressManager::addTask(m_future, tr("Evaluating Type Hierarchy"), "TypeHierarchy");
 }
 
 void CppTypeHierarchyWidget::displayHierarchy()
 {
+    updateSynchronizer();
     hideProgress();
     clearTypeHierarchy();
 

src/plugins/cppeditor/cpptypehierarchy.h

@@ -29,6 +29,7 @@
 
 #include <QFuture>
 #include <QFutureWatcher>
+#include <QFutureSynchronizer>
 #include <QList>
 #include <QSharedPointer>
 #include <QStackedWidget>
@@ -75,7 +76,6 @@ class CppTypeHierarchyWidget : public QWidget
     Q_OBJECT
 public:
     CppTypeHierarchyWidget();
-    ~CppTypeHierarchyWidget() override;
 
     void perform();
 
@@ -92,6 +92,7 @@ private:
     void hideProgress();
     void clearTypeHierarchy();
     void onItemActivated(const QModelIndex &index);
+    void updateSynchronizer();
 
     CppEditorWidget *m_cppEditor = nullptr;
     Utils::NavigationTreeView *m_treeView = nullptr;
@@ -102,7 +103,8 @@ private:
     TextEditor::TextEditorLinkLabel *m_inspectedClass = nullptr;
     QLabel *m_infoLabel = nullptr;
     QFuture<QSharedPointer<CppTools::CppElement>> m_future;
-    QFutureWatcher<QSharedPointer<CppTools::CppElement>> m_futureWatcher;
+    QFutureWatcher<void> m_futureWatcher;
+    QFutureSynchronizer<void> m_synchronizer;
     Utils::ProgressIndicator *m_progressIndicator = nullptr;
 };
 

src/plugins/cpptools/cppelementevaluator.cpp

@@ -214,6 +214,8 @@ void CppClass::lookupDerived(QFutureInterfaceBase &futureInterface,
     using Data = QPair<CppClass*, CppTools::TypeHierarchy>;
 
     snapshot.updateDependencyTable(futureInterface);
+    if (futureInterface.isCanceled())
+        return;
     CppTools::TypeHierarchyBuilder builder(declaration, snapshot);
     const CppTools::TypeHierarchy &completeHierarchy = builder.buildDerivedTypeHierarchy(futureInterface);
 
@@ -531,7 +533,7 @@ static void handleLookupItemMatch(QFutureInterface<QSharedPointer<CppElement>> &
 
             if (futureInterface.isCanceled())
                 return;
-            auto cppClass = new CppClass(declaration);
+            QSharedPointer<CppClass> cppClass(new CppClass(declaration));
             if (lookupBaseClasses)
                 cppClass->lookupBases(futureInterface, declaration, contextToUse);
             if (futureInterface.isCanceled())
@@ -540,7 +542,7 @@ static void handleLookupItemMatch(QFutureInterface<QSharedPointer<CppElement>> &
                 cppClass->lookupDerived(futureInterface, declaration, snapshot);
             if (futureInterface.isCanceled())
                 return;
-            element = QSharedPointer<CppElement>(cppClass);
+            element = cppClass;
         } else if (Enum *enumDecl = declaration->asEnum()) {
             element = QSharedPointer<CppElement>(new CppEnum(enumDecl));
         } else if (auto enumerator = dynamic_cast<EnumeratorDeclaration *>(declaration)) {