From 06838e3e5e41f1f090fdd016c736bb91241cd5b3 Mon Sep 17 00:00:00 2001
From: Marcus Tillmanns <marcus.tillmanns@qt.io>
Date: Tue, 8 Nov 2022 15:08:21 +0100
Subject: [PATCH] ProjectExplorer: Fix potential race condition

When using BuildStep::runImpl() it was possible for the async part
to still be running while the BuildStep is already deleted.

This change returns the Future so that users can wait for it to finish.

Change-Id: I27c0fc8741c59851c5ab8f5cb858fbcda923c14d
Reviewed-by: Jarek Kobus <jaroslaw.kobus@qt.io>
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>
---
 src/plugins/android/androiddeployqtstep.cpp        | 2 +-
 src/plugins/android/androiddeployqtstep.h          | 3 +++
 src/plugins/projectexplorer/buildstep.cpp          | 6 ++++--
 src/plugins/projectexplorer/buildstep.h            | 3 ++-
 src/plugins/remotelinux/tarpackagecreationstep.cpp | 6 +++++-
 5 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/src/plugins/android/androiddeployqtstep.cpp b/src/plugins/android/androiddeployqtstep.cpp
index d3fedd7b055..0d33bce4c0f 100644
--- a/src/plugins/android/androiddeployqtstep.cpp
+++ b/src/plugins/android/androiddeployqtstep.cpp
@@ -488,7 +488,7 @@ void AndroidDeployQtStep::gatherFilesToPull()
 
 void AndroidDeployQtStep::doRun()
 {
-    runInThread([this] { return runImpl(); });
+    m_synchronizer.addFuture(runInThread([this] { return runImpl(); }));
 }
 
 void AndroidDeployQtStep::runCommand(const CommandLine &command)
diff --git a/src/plugins/android/androiddeployqtstep.h b/src/plugins/android/androiddeployqtstep.h
index f11276657b2..2f172615951 100644
--- a/src/plugins/android/androiddeployqtstep.h
+++ b/src/plugins/android/androiddeployqtstep.h
@@ -12,6 +12,7 @@
 
 #include <utils/commandline.h>
 #include <utils/environment.h>
+#include <utils/futuresynchronizer.h>
 
 namespace Utils { class QtcProcess; }
 
@@ -91,6 +92,8 @@ private:
     Utils::FilePath m_workingDirectory;
     Utils::Environment m_environment;
     AndroidDeviceInfo m_deviceInfo;
+
+    Utils::FutureSynchronizer m_synchronizer;
 };
 
 }
diff --git a/src/plugins/projectexplorer/buildstep.cpp b/src/plugins/projectexplorer/buildstep.cpp
index 070e8f3b545..4088ce3d0b4 100644
--- a/src/plugins/projectexplorer/buildstep.cpp
+++ b/src/plugins/projectexplorer/buildstep.cpp
@@ -295,7 +295,7 @@ QVariant BuildStep::data(Id id) const
     immutable steps are run. The default implementation returns \c false.
 */
 
-void BuildStep::runInThread(const std::function<bool()> &syncImpl)
+QFuture<bool> BuildStep::runInThread(const std::function<bool()> &syncImpl)
 {
     m_runInGuiThread = false;
     m_cancelFlag = false;
@@ -304,7 +304,9 @@ void BuildStep::runInThread(const std::function<bool()> &syncImpl)
         emit finished(watcher->result());
         watcher->deleteLater();
     });
-    watcher->setFuture(Utils::runAsync(syncImpl));
+    auto future = Utils::runAsync(syncImpl);
+    watcher->setFuture(future);
+    return future;
 }
 
 std::function<bool ()> BuildStep::cancelChecker() const
diff --git a/src/plugins/projectexplorer/buildstep.h b/src/plugins/projectexplorer/buildstep.h
index 79ae4a60bce..0f5def37bd4 100644
--- a/src/plugins/projectexplorer/buildstep.h
+++ b/src/plugins/projectexplorer/buildstep.h
@@ -10,6 +10,7 @@
 
 #include <utils/qtcassert.h>
 
+#include <QFuture>
 #include <QWidget>
 
 #include <atomic>
@@ -117,7 +118,7 @@ signals:
 protected:
     virtual QWidget *createConfigWidget();
 
-    void runInThread(const std::function<bool()> &syncImpl);
+    QFuture<bool> runInThread(const std::function<bool()> &syncImpl);
 
     std::function<bool()> cancelChecker() const;
     bool isCanceled() const;
diff --git a/src/plugins/remotelinux/tarpackagecreationstep.cpp b/src/plugins/remotelinux/tarpackagecreationstep.cpp
index 9255c2e2287..6f33f70e665 100644
--- a/src/plugins/remotelinux/tarpackagecreationstep.cpp
+++ b/src/plugins/remotelinux/tarpackagecreationstep.cpp
@@ -13,6 +13,8 @@
 #include <projectexplorer/project.h>
 #include <projectexplorer/target.h>
 
+#include <utils/futuresynchronizer.h>
+
 #include <QDateTime>
 #include <QDir>
 #include <QFile>
@@ -80,6 +82,8 @@ private:
     BoolAspect *m_ignoreMissingFilesAspect = nullptr;
     bool m_packagingNeeded = false;
     QList<DeployableFile> m_files;
+
+    FutureSynchronizer m_synchronizer;
 };
 
 TarPackageCreationStep::TarPackageCreationStep(BuildStepList *bsl, Id id)
@@ -126,7 +130,7 @@ bool TarPackageCreationStep::init()
 
 void TarPackageCreationStep::doRun()
 {
-    runInThread([this] { return runImpl(); });
+    m_synchronizer.addFuture(runInThread([this] { return runImpl(); }));
 }
 
 bool TarPackageCreationStep::fromMap(const QVariantMap &map)