From d95fa019d4bff2bbdcf148c0096970f8aca4b585 Mon Sep 17 00:00:00 2001
From: Jarek Kobus <jaroslaw.kobus@qt.io>
Date: Thu, 15 Feb 2024 17:59:57 +0100
Subject: [PATCH] PythonSettings: Don't leak running watchers

Detected by memory analyzer.
When shutdown comes while pythonsFromRegistry() or
pythonsFromPath() is still running, the corresponding
QFutureWatcher is leaked.

Employ TaskTreeRunner instead. It handles the cancellation
of the running tasks automatically on its destruction.

Make pythonsFromRegistry() and pythonsFromPath() cancelable,
by providing QPromise as a parameter and check for canceled
state on every iteration.

Change-Id: Iae7c7d1ed764646b8203bd7ca8b9580cb999b80c
Reviewed-by: David Schulz <david.schulz@qt.io>
---
 src/plugins/python/pythonsettings.cpp | 81 +++++++++++++--------------
 src/plugins/python/pythonsettings.h   |  3 +
 2 files changed, 43 insertions(+), 41 deletions(-)

diff --git a/src/plugins/python/pythonsettings.cpp b/src/plugins/python/pythonsettings.cpp
index f2f5975ff40..d9f5f74e9ff 100644
--- a/src/plugins/python/pythonsettings.cpp
+++ b/src/plugins/python/pythonsettings.cpp
@@ -53,9 +53,9 @@
 #include <QVBoxLayout>
 #include <QWidget>
 
+using namespace Layouting;
 using namespace ProjectExplorer;
 using namespace Utils;
-using namespace Layouting;
 
 namespace Python::Internal {
 
@@ -645,12 +645,15 @@ void PythonSettings::disableOutdatedPyls()
     }
 }
 
-static QList<Interpreter> pythonsFromRegistry()
+static void pythonsFromRegistry(QPromise<QList<Interpreter>> &promise)
 {
     QList<Interpreter> pythons;
     QSettings pythonRegistry("HKEY_LOCAL_MACHINE\\SOFTWARE\\Python\\PythonCore",
                              QSettings::NativeFormat);
     for (const QString &versionGroup : pythonRegistry.childGroups()) {
+        if (promise.isCanceled())
+            return;
+
         pythonRegistry.beginGroup(versionGroup);
         QString name = pythonRegistry.value("DisplayName").toString();
         QVariant regVal = pythonRegistry.value("InstallPath/ExecutablePath");
@@ -671,14 +674,17 @@ static QList<Interpreter> pythonsFromRegistry()
         }
         pythonRegistry.endGroup();
     }
-    return pythons;
+    promise.addResult(pythons);
 }
 
-static QList<Interpreter> pythonsFromPath()
+static void pythonsFromPath(QPromise<QList<Interpreter>> &promise)
 {
     QList<Interpreter> pythons;
     if (HostOsInfo::isWindowsHost()) {
         for (const FilePath &executable : FilePath("python").searchAllInPath()) {
+            if (promise.isCanceled())
+                return;
+
             // Windows creates empty redirector files that may interfere
             if (executable.toFileInfo().size() == 0)
                 continue;
@@ -695,6 +701,9 @@ static QList<Interpreter> pythonsFromPath()
         for (const FilePath &path : dirs) {
             const QDir dir(path.toString());
             for (const QFileInfo &fi : dir.entryInfoList(filters)) {
+                if (promise.isCanceled())
+                    return;
+
                 const FilePath executable = FilePath::fromUserInput(fi.canonicalFilePath());
                 if (!used.contains(executable) && executable.exists()) {
                     used.insert(executable);
@@ -703,7 +712,7 @@ static QList<Interpreter> pythonsFromPath()
             }
         }
     }
-    return pythons;
+    promise.addResult(pythons);
 }
 
 static QString idForPythonFromPath(const QList<Interpreter> &pythons)
@@ -732,41 +741,6 @@ static bool alreadyRegistered(const Interpreter &candidate)
                         });
 }
 
-static void scanPath()
-{
-    auto watcher = new QFutureWatcher<QList<Interpreter>>();
-    QObject::connect(watcher, &QFutureWatcher<QList<Interpreter>>::finished, [watcher]() {
-        for (const Interpreter &interpreter : watcher->result()) {
-            if (!alreadyRegistered(interpreter))
-                settingsInstance->addInterpreter(interpreter);
-        }
-        watcher->deleteLater();
-    });
-    watcher->setFuture(Utils::asyncRun(pythonsFromPath));
-}
-
-static void scanRegistry()
-{
-    auto watcher = new QFutureWatcher<QList<Interpreter>>();
-    QObject::connect(watcher, &QFutureWatcher<QList<Interpreter>>::finished, [watcher]() {
-        for (const Interpreter &interpreter : watcher->result()) {
-            if (!alreadyRegistered(interpreter))
-                settingsInstance->addInterpreter(interpreter);
-        }
-        watcher->deleteLater();
-        scanPath();
-    });
-    watcher->setFuture(Utils::asyncRun(pythonsFromRegistry));
-}
-
-static void scanSystemForInterpreters()
-{
-    if (Utils::HostOsInfo::isWindowsHost())
-        scanRegistry();
-    else
-        scanPath();
-}
-
 PythonSettings::PythonSettings()
 {
     QTC_ASSERT(!settingsInstance, return);
@@ -777,7 +751,32 @@ PythonSettings::PythonSettings()
 
     initFromSettings(Core::ICore::settings());
 
-    scanSystemForInterpreters();
+    const auto onRegistrySetup = [](Async<QList<Interpreter>> &task) {
+        task.setFutureSynchronizer(ExtensionSystem::PluginManager::futureSynchronizer());
+        task.setConcurrentCallData(pythonsFromRegistry);
+    };
+    const auto onPathSetup = [](Async<QList<Interpreter>> &task) {
+        task.setFutureSynchronizer(ExtensionSystem::PluginManager::futureSynchronizer());
+        task.setConcurrentCallData(pythonsFromPath);
+    };
+    const auto onTaskDone = [](const Async<QList<Interpreter>> &task) {
+        if (!task.isResultAvailable())
+            return;
+
+        const auto interpreters = task.result();
+        for (const Interpreter &interpreter : interpreters) {
+            if (!alreadyRegistered(interpreter))
+                settingsInstance->addInterpreter(interpreter);
+        }
+    };
+
+    const Tasking::Group recipe {
+        Tasking::finishAllAndSuccess,
+        Utils::HostOsInfo::isWindowsHost()
+            ? AsyncTask<QList<Interpreter>>(onRegistrySetup, onTaskDone) : Tasking::GroupItem({}),
+        AsyncTask<QList<Interpreter>>(onPathSetup, onTaskDone)
+    };
+    m_taskTreeRunner.start(recipe);
 
     if (m_defaultInterpreterId.isEmpty())
         m_defaultInterpreterId = idForPythonFromPath(m_interpreters);
diff --git a/src/plugins/python/pythonsettings.h b/src/plugins/python/pythonsettings.h
index 892069431d5..ab5d092a5ff 100644
--- a/src/plugins/python/pythonsettings.h
+++ b/src/plugins/python/pythonsettings.h
@@ -5,6 +5,8 @@
 
 #include <projectexplorer/runconfigurationaspects.h>
 
+#include <solutions/tasking/tasktreerunner.h>
+
 #include <utils/filepath.h>
 
 namespace Python::Internal {
@@ -69,6 +71,7 @@ private:
     QString m_defaultInterpreterId;
     bool m_pylsEnabled = true;
     QString m_pylsConfiguration;
+    Tasking::TaskTreeRunner m_taskTreeRunner;
 
     static void saveSettings();
 };