LinuxDeviceTester: Fix a crash on closing dialog when tryToConnect()

Make a call to tryToConnect() a yet another task inside the task tree.
Prolong the lifetime of the linux device until the tryToConnect()
running in a separate thread is finished. This fixes a crash
when canceling the test and closing the preferences dialog while
tryToConnect() is still running.

Amends e8ad29d3af

Change-Id: I5aca6c9c3de0792421b9774faf45cd463854265e
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: hjk <hjk@qt.io>

src/plugins/remotelinux/linuxdevice.cpp

@@ -24,7 +24,6 @@
 #include <projectexplorer/projectexplorerconstants.h>
 
 #include <utils/algorithm.h>
-#include <utils/async.h>
 #include <utils/devicefileaccess.h>
 #include <utils/deviceshell.h>
 #include <utils/environment.h>
@@ -1674,12 +1673,10 @@ void LinuxDevice::setDisconnected(bool disconnected)
     d->setDisconnected(disconnected);
 }
 
-QFuture<bool> LinuxDevice::tryToConnect()
+bool LinuxDevice::tryToConnect()
 {
-    return Utils::asyncRun([this] {
+    QMutexLocker locker(&d->m_shellMutex);
-        QMutexLocker locker(&d->m_shellMutex);
+    return d->setupShell(sshParameters(), false);
-        return d->setupShell(sshParameters(), false);
-    });
 }
 
 namespace Internal {

src/plugins/remotelinux/linuxdevice.h

@@ -50,7 +50,7 @@ public:
     bool isDisconnected() const;
     void setDisconnected(bool disconnected);
 
-    QFuture<bool> tryToConnect();
+    bool tryToConnect();
 
 protected:
     LinuxDevice();

src/plugins/remotelinux/linuxdevicetester.cpp

@@ -6,6 +6,8 @@
 #include "linuxdevice.h"
 #include "remotelinuxtr.h"
 
+#include <extensionsystem/pluginmanager.h>
+
 #include <projectexplorer/devicesupport/deviceusedportsgatherer.h>
 #include <projectexplorer/devicesupport/filetransfer.h>
 #include <projectexplorer/projectexplorerconstants.h>
@@ -13,13 +15,12 @@
 #include <solutions/tasking/tasktreerunner.h>
 
 #include <utils/algorithm.h>
+#include <utils/async.h>
 #include <utils/process.h>
 #include <utils/processinterface.h>
 #include <utils/qtcassert.h>
 #include <utils/stringutils.h>
 
-#include <QFutureWatcher>
-
 using namespace ProjectExplorer;
 using namespace Tasking;
 using namespace Utils;
@@ -39,6 +40,7 @@ public:
 
     QStringList commandsToTest() const;
 
+    GroupItem connectionTask() const;
     GroupItem echoTask(const QString &contents) const;
     GroupItem unameTask() const;
     GroupItem gathererTask() const;
@@ -46,13 +48,9 @@ public:
                            const Storage<TransferStorage> &storage) const;
     GroupItem transferTasks() const;
     GroupItem commandTasks() const;
-    void runCommandTests();
-
-    bool isRunning() const { return m_connectionTest || m_taskTreeRunner.isRunning(); }
 
     GenericLinuxDeviceTester *q = nullptr;
     LinuxDevice::Ptr m_device;
-    QFutureWatcher<bool> *m_connectionTest = nullptr;
     TaskTreeRunner m_taskTreeRunner;
     QStringList m_extraCommands;
     QList<GroupItem> m_extraTests;
@@ -98,6 +96,27 @@ QStringList GenericLinuxDeviceTesterPrivate::commandsToTest() const
     return commands;
 }
 
+GroupItem GenericLinuxDeviceTesterPrivate::connectionTask() const
+{
+    const auto onSetup = [this](Async<bool> &task) {
+        emit q->progressMessage(Tr::tr("Connecting to device..."));
+        task.setConcurrentCallData([device = m_device] { return device->tryToConnect(); });
+        task.setFutureSynchronizer(ExtensionSystem::PluginManager::futureSynchronizer());
+    };
+    const auto onDone = [this](const Async<bool> &task) {
+        const bool success = task.isResultAvailable() && task.result();
+        if (success) {
+            // TODO: For master: move the '\n' outside of Tr().
+            emit q->progressMessage(Tr::tr("Connected. Now doing extended checks.\n"));
+        } else {
+            emit q->errorMessage(
+                Tr::tr("Basic connectivity test failed, device is considered unusable.") + '\n');
+        }
+        return toDoneResult(success);
+    };
+    return AsyncTask<bool>(onSetup, onDone);
+}
+
 GroupItem GenericLinuxDeviceTesterPrivate::echoTask(const QString &contents) const
 {
     const auto onSetup = [this, contents](Process &process) {
@@ -283,20 +302,6 @@ GroupItem GenericLinuxDeviceTesterPrivate::commandTasks() const
     return root;
 }
 
-void GenericLinuxDeviceTesterPrivate::runCommandTests()
-{
-    const Group root {
-        echoTask("Hello"), // No quoting necessary
-        echoTask("Hello Remote World!"), // Checks quoting, too.
-        unameTask(),
-        gathererTask(),
-        transferTasks(),
-        m_extraTests,
-        commandTasks()
-    };
-    m_taskTreeRunner.start(root);
-}
-
 } // namespace Internal
 
 using namespace Internal;
@@ -323,39 +328,27 @@ void GenericLinuxDeviceTester::setExtraTests(const QList<GroupItem> &extraTests)
 
 void GenericLinuxDeviceTester::testDevice(const IDevice::Ptr &deviceConfiguration)
 {
-    QTC_ASSERT(!d->isRunning(), return);
+    QTC_ASSERT(!d->m_taskTreeRunner.isRunning(), return);
-
-    emit progressMessage(Tr::tr("Connecting to device..."));
 
     d->m_device = std::static_pointer_cast<LinuxDevice>(deviceConfiguration);
 
-    d->m_connectionTest = new QFutureWatcher<bool>(this);
+    const Group root {
-    connect(d->m_connectionTest, &QFutureWatcher<bool>::finished, this, [this] {
+        d->connectionTask(),
-        const bool success = d->m_connectionTest->result();
+        d->echoTask("Hello"), // No quoting necessary
-        d->m_connectionTest->deleteLater();
+        d->echoTask("Hello Remote World!"), // Checks quoting, too.
-        d->m_connectionTest = nullptr;
+        d->unameTask(),
-        if (success) {
+        d->gathererTask(),
-            emit progressMessage(Tr::tr("Connected. Now doing extended checks.\n"));
+        d->transferTasks(),
-            d->runCommandTests();
+        d->m_extraTests,
-        } else {
+        d->commandTasks()
-            emit errorMessage(
+    };
-                Tr::tr("Basic connectivity test failed, device is considered unusable."));
+    d->m_taskTreeRunner.start(root);
-            emit finished(TestFailure);
-        }
-    });
-    d->m_connectionTest->setFuture(d->m_device->tryToConnect());
 }
 
 void GenericLinuxDeviceTester::stopTest()
 {
-    QTC_ASSERT(d->isRunning(), return);
+    QTC_ASSERT(d->m_taskTreeRunner.isRunning(), return);
-    if (d->m_connectionTest) {
+    d->m_taskTreeRunner.reset();
-        d->m_connectionTest->disconnect();
-        d->m_connectionTest->cancel();
-        d->m_connectionTest = nullptr;
-    } else {
-        d->m_taskTreeRunner.reset();
-    }
     emit finished(TestFailure);
 }