ProjectExplorer: Run RunControl::{start,stop} always asynchronously

This introduces a mini-state-"machine" to handle RunControl states
Intialized->[Starting->Running->Stopping->Stopped->]*->Finished.

Needing time between trying to start and getting feedback is nowadays
the normal setup for all remote targets as well as for most local tools.
Making that the default for all runs simplifies the code and provides an
opportunity to (a) fix some currently wrong reports of "stopped
immediately" and (b) to remove target-specific (WinRT) or tool-specific
(Valgrind, GammaRay) state members doing essentially the same.

Change-Id: I7f52fee41144188ee8389e922fdc265f8c0a6459
Reviewed-by: David Schulz <david.schulz@qt.io>
Reviewed-by: Tobias Hunger <tobias.hunger@qt.io>

src/plugins/android/androidruncontrol.cpp

@@ -67,10 +67,9 @@ void AndroidRunControl::start()
     m_runner->start();
 }
 
-RunControl::StopResult AndroidRunControl::stop()
+void AndroidRunControl::stop()
 {
     m_runner->stop();
-    return StoppedSynchronously;
 }
 
 void AndroidRunControl::handleRemoteProcessFinished(const QString &error)

src/plugins/android/androidruncontrol.h

@@ -42,7 +42,7 @@ public:
     ~AndroidRunControl() override;
 
     void start() override;
-    StopResult stop() override;
+    void stop() override;
     QString displayName() const override;
 
 private:

src/plugins/clangstaticanalyzer/clangstaticanalyzerruncontrol.cpp

@@ -563,7 +563,7 @@ void ClangStaticAnalyzerRunControl::start()
         analyzeNextFile();
 }
 
-RunControl::StopResult ClangStaticAnalyzerRunControl::stop()
+void ClangStaticAnalyzerRunControl::stop()
 {
     QSetIterator<ClangStaticAnalyzerRunner *> i(m_runners);
     while (i.hasNext()) {
@@ -577,7 +577,6 @@ RunControl::StopResult ClangStaticAnalyzerRunControl::stop()
                   Utils::NormalMessageFormat);
     m_progress.reportFinished();
     reportApplicationStop();
-    return RunControl::StoppedSynchronously;
 }
 
 void ClangStaticAnalyzerRunControl::analyzeNextFile()

src/plugins/clangstaticanalyzer/clangstaticanalyzerruncontrol.h

@@ -57,7 +57,7 @@ public:
                                   const CppTools::ProjectInfo &projectInfo);
 
     void start() override;
-    StopResult stop() override;
+    void stop() override;
 
     bool success() const { return m_success; } // For testing.
     bool supportsReRunning() const override { return false; }

src/plugins/debugger/debuggerruncontrol.cpp

@@ -276,11 +276,10 @@ bool DebuggerRunControl::promptToStop(bool *optionalPrompt) const
     return result;
 }
 
-RunControl::StopResult DebuggerRunControl::stop()
+void DebuggerRunControl::stop()
 {
-    QTC_ASSERT(m_engine, return StoppedSynchronously);
+    QTC_ASSERT(m_engine, return);
     m_engine->quitDebugger();
-    return AsynchronousStop;
 }
 
 void DebuggerRunControl::debuggingFinished()

src/plugins/debugger/debuggerruncontrol.h

@@ -68,7 +68,7 @@ public:
     // ProjectExplorer::RunControl
     void start() override;
     bool promptToStop(bool *prompt = 0) const override;
-    StopResult stop() override; // Called from SnapshotWindow.
+    void stop() override; // Called from SnapshotWindow.
     QString displayName() const override;
     bool supportsReRunning() const override;
     void handleApplicationOutput(const QString &msg, int channel);

src/plugins/ios/iosruncontrol.cpp

@@ -64,10 +64,9 @@ void IosRunControl::start()
     m_runner->start();
 }
 
-RunControl::StopResult IosRunControl::stop()
+void IosRunControl::stop()
 {
     m_runner->stop();
-    return StoppedSynchronously;
 }
 
 void IosRunControl::handleRemoteProcessFinished(bool cleanEnd)

src/plugins/ios/iosruncontrol.h

@@ -41,9 +41,8 @@ public:
     explicit IosRunControl(IosRunConfiguration *runConfig);
     ~IosRunControl() override;
 
-
     void start() override;
-    StopResult stop() override;
+    void stop() override;
     QString displayName() const override;
 
 private:

src/plugins/ios/iosrunfactories.cpp

@@ -179,7 +179,7 @@ RunControl *IosRunControlFactory::create(RunConfiguration *runConfig,
     // The device can only run an application at a time, if an app is running stop it.
     if (m_activeRunControls.contains(devId)) {
         if (QPointer<RunControl> activeRunControl = m_activeRunControls[devId])
-            activeRunControl->stop();
+            activeRunControl->initiateStop();
         m_activeRunControls.remove(devId);
     }
     if (mode == ProjectExplorer::Constants::NORMAL_RUN_MODE)

src/plugins/projectexplorer/appoutputpane.cpp

@@ -506,7 +506,7 @@ void AppOutputPane::reRunRunControl()
 
     handleOldOutput(tab.window);
     tab.window->scrollToBottom();
-    tab.runControl->start();
+    tab.runControl->initiateStart();
 }
 
 void AppOutputPane::attachToRunControl()
@@ -525,7 +525,7 @@ void AppOutputPane::stopRunControl()
 
     RunControl *rc = m_runControlTabs.at(index).runControl;
     if (rc->isRunning() && optionallyPromptToStop(rc))
-        rc->stop();
+        rc->initiateStop();
 
     if (debug)
         qDebug() << "OutputPane::stopRunControl " << rc;
@@ -556,7 +556,7 @@ bool AppOutputPane::closeTab(int tabIndex, CloseTabMode closeTabMode)
 
     if (debug)
         qDebug() << "OutputPane::closeTab tab " << tabIndex << m_runControlTabs[index].runControl
-                        << m_runControlTabs[index].window << m_runControlTabs[index].asyncClosing;
+                 << m_runControlTabs[index].window;
     // Prompt user to stop
     if (m_runControlTabs[index].runControl->isRunning()) {
         switch (closeTabMode) {
@@ -575,15 +575,8 @@ bool AppOutputPane::closeTab(int tabIndex, CloseTabMode closeTabMode)
             break;
         }
         if (m_runControlTabs[index].runControl->isRunning()) { // yes it might have stopped already, then just close
-            QWidget *tabWidget = m_tabWidget->widget(tabIndex);
+            m_runControlTabs[index].runControl->initiateStop();
-            if (m_runControlTabs[index].runControl->stop() == RunControl::AsynchronousStop) {
+            return false;
-                m_runControlTabs[index].asyncClosing = true;
-                return false;
-            }
-            tabIndex = m_tabWidget->indexOf(tabWidget);
-            index = indexOf(tabWidget);
-            if (tabIndex == -1 || index == -1)
-                return false;
         }
     }
 
@@ -741,10 +734,6 @@ void AppOutputPane::slotRunControlFinished2(RunControl *sender)
 
     m_runControlTabs.at(senderIndex).window->setFormatter(nullptr); // Reset formater for this RC
 
-    // Check for asynchronous close. Close the tab.
-    if (m_runControlTabs.at(senderIndex).asyncClosing)
-        closeTab(tabWidgetIndexOf(senderIndex), CloseTabNoPrompt);
-
     emit runControlFinished(sender);
 
     if (!isRunning())

src/plugins/projectexplorer/appoutputpane.h

@@ -128,8 +128,6 @@ private:
                                Core::OutputWindow *window = nullptr);
         RunControl *runControl;
         Core::OutputWindow *window;
-        // Is the run control stopping asynchronously, close the tab once it finishes
-        bool asyncClosing = false;
         BehaviorOnOutput behaviorOnOutput = Flash;
     };
 

src/plugins/projectexplorer/projectexplorer.cpp

@@ -2004,7 +2004,7 @@ void ProjectExplorerPluginPrivate::startRunControl(RunControl *runControl, Core:
             || ((runMode == Constants::DEBUG_RUN_MODE || runMode == Constants::DEBUG_RUN_MODE_WITH_BREAK_ON_MAIN)
                 && m_projectExplorerSettings.showDebugOutput);
     m_outputPane->setBehaviorOnOutput(runControl, popup ? AppOutputPane::Popup : AppOutputPane::Flash);
-    runControl->start();
+    runControl->initiateStart();
     emit m_instance->updateRunActions();
 }
 
@@ -2306,16 +2306,11 @@ int ProjectExplorerPluginPrivate::queue(QList<Project *> projects, QList<Id> ste
                 }
             }
 
-            QList<RunControl *> asyncStop;
             if (stopThem) {
-                foreach (RunControl *rc, toStop) {
+                foreach (RunControl *rc, toStop)
-                    if (rc->stop() == RunControl::AsynchronousStop)
+                    rc->initiateStop();
-                        asyncStop << rc;
-                }
-            }
 
-            if (!asyncStop.isEmpty()) {
+                WaitForStopDialog dialog(toStop);
-                WaitForStopDialog dialog(asyncStop);
                 dialog.exec();
 
                 if (dialog.canceled())

src/plugins/projectexplorer/runconfiguration.cpp

@@ -535,7 +535,7 @@ public:
     // A handle to the actual application process.
     Utils::ProcessHandle applicationProcessHandle;
 
-    bool isRunning = false;
+    RunControl::State state = RunControl::State::Initialized;
 
 #ifdef Q_OS_OSX
     // This is used to bring apps in the foreground on Mac
@@ -575,6 +575,18 @@ RunControl::~RunControl()
     delete d;
 }
 
+void RunControl::initiateStart()
+{
+    setState(State::Starting);
+    QTimer::singleShot(0, this, &RunControl::start);
+}
+
+void RunControl::initiateStop()
+{
+    setState(State::Stopping);
+    QTimer::singleShot(0, this, &RunControl::stop);
+}
+
 Utils::OutputFormatter *RunControl::outputFormatter() const
 {
     return d->outputFormatter;
@@ -696,7 +708,7 @@ bool RunControl::promptToStop(bool *optionalPrompt) const
 
 bool RunControl::isRunning() const
 {
-    return d->isRunning;
+    return d->state == State::Running;
 }
 
 /*!
@@ -735,6 +747,33 @@ bool RunControl::showPromptToStopDialog(const QString &title,
     return close;
 }
 
+static bool isAllowedTransition(RunControl::State from, RunControl::State to)
+{
+    switch (from) {
+    case RunControl::State::Initialized:
+        return to == RunControl::State::Starting;
+    case RunControl::State::Starting:
+        return to == RunControl::State::Running;
+    case RunControl::State::Running:
+        return to == RunControl::State::Stopping
+            || to == RunControl::State::Stopped;
+    case RunControl::State::Stopping:
+        return to == RunControl::State::Stopped;
+    case RunControl::State::Stopped:
+        return false;
+    }
+    qDebug() << "UNKNOWN DEBUGGER STATE:" << from;
+    return false;
+}
+
+void RunControl::setState(RunControl::State state)
+{
+    if (!isAllowedTransition(d->state, state)) {
+        qDebug() << "Invalid run state transition from " << d->state << " to " << state;
+    }
+    d->state = state;
+}
+
 /*!
     Brings the application determined by this RunControl's \c applicationProcessHandle
     to the foreground.
@@ -752,13 +791,18 @@ void RunControl::bringApplicationToForeground()
 
 void RunControl::reportApplicationStart()
 {
-    d->isRunning = true;
+    setState(State::Running);
     emit started(QPrivateSignal());
 }
 
 void RunControl::reportApplicationStop()
 {
-    d->isRunning = false;
+    if (d->state == State::Stopped) {
+        // FIXME: Currently various tool implementations call reportApplicationStop()
+        // multiple times. Fix it there and then add a soft assert here.
+        return;
+    }
+    setState(State::Stopped);
     QTC_CHECK(d->applicationProcessHandle.isValid());
     setApplicationProcessHandle(Utils::ProcessHandle());
     emit finished(QPrivateSignal());
@@ -896,14 +940,9 @@ void SimpleRunControl::start()
     }
 }
 
-RunControl::StopResult SimpleRunControl::stop()
+void SimpleRunControl::stop()
 {
     d->m_launcher.stop();
-
-    if (isSynchronousLauncher(this))
-        return StoppedSynchronously;
-    else
-        return AsynchronousStop;
 }
 
 void SimpleRunControl::onProcessStarted()

src/plugins/projectexplorer/runconfiguration.h

@@ -353,20 +353,24 @@ class PROJECTEXPLORER_EXPORT RunControl : public QObject
     Q_OBJECT
 
 public:
-    enum StopResult {
+    enum class State {
-        StoppedSynchronously, // Stopped.
+        Initialized,
-        AsynchronousStop     // Stop sequence has been started
+        Starting,
+        Running,
+        Stopping,
+        Stopped
     };
+    Q_ENUM(State)
 
     RunControl(RunConfiguration *runConfiguration, Core::Id mode);
     ~RunControl() override;
-    virtual void start() = 0;
+
+    void initiateStart(); // Calls start() asynchronously.
+    void initiateStop(); // Calls stop() asynchronously.
 
     virtual bool promptToStop(bool *optionalPrompt = nullptr) const;
-    virtual StopResult stop() = 0;
     virtual bool supportsReRunning() const { return true; }
 
-
     virtual QString displayName() const;
     void setDisplayName(const QString &displayName);
 
@@ -404,6 +408,9 @@ signals:
     void applicationProcessHandleChanged(QPrivateSignal); // Use setApplicationProcessHandle
 
 protected:
+    virtual void start() = 0;
+    virtual void stop() = 0;
+
     void reportApplicationStart(); // Call this when the application starts to run
     void reportApplicationStop(); // Call this when the application has stopped for any reason
 
@@ -413,6 +420,7 @@ protected:
                                 bool *prompt = nullptr) const;
 
 private:
+    void setState(State state);
     void bringApplicationToForegroundInternal();
     Internal::RunControlPrivate *d;
 };
@@ -425,7 +433,7 @@ public:
 
     ApplicationLauncher &applicationLauncher();
     void start() override;
-    StopResult stop() override;
+    void stop() override;
 
     virtual void onProcessStarted();
     virtual void onProcessFinished(int exitCode, QProcess::ExitStatus status);

src/plugins/qmlprofiler/qmlprofilerruncontrol.cpp

@@ -118,9 +118,9 @@ void QmlProfilerRunControl::start()
     emit starting();
 }
 
-RunControl::StopResult QmlProfilerRunControl::stop()
+void QmlProfilerRunControl::stop()
 {
-    QTC_ASSERT(d->m_profilerState, return RunControl::StoppedSynchronously);
+    QTC_ASSERT(d->m_profilerState, return);
 
     switch (d->m_profilerState->currentState()) {
     case QmlProfilerStateManager::AppRunning:
@@ -141,8 +141,6 @@ RunControl::StopResult QmlProfilerRunControl::stop()
     }
         break;
     }
-
-    return RunControl::StoppedSynchronously;
 }
 
 void QmlProfilerRunControl::notifyRemoteFinished()

src/plugins/qmlprofiler/qmlprofilerruncontrol.h

@@ -48,7 +48,7 @@ public:
     void notifyRemoteSetupDone(Utils::Port port) override;
     void notifyRemoteSetupFailed(const QString &errorMessage) override;
     void start() override;
-    StopResult stop() override;
+    void stop() override;
     void cancelProcess();
     void notifyRemoteFinished() override;
     bool supportsReRunning() const override { return false; }

src/plugins/qmlprofiler/tests/localqmlprofilerrunner_test.cpp

@@ -37,40 +37,39 @@ LocalQmlProfilerRunnerTest::LocalQmlProfilerRunnerTest(QObject *parent) : QObjec
 {
 }
 
+void LocalQmlProfilerRunnerTest::connectRunner(LocalQmlProfilerRunner *runner)
+{
+    connect(runner, &LocalQmlProfilerRunner::started, this, [this] {
+        QVERIFY(!running);
+        ++runCount;
+        running = true;
+    });
+    connect(runner, &LocalQmlProfilerRunner::stopped, this, [this] {
+        QVERIFY(running);
+        running = false;
+    });
+}
+
 void LocalQmlProfilerRunnerTest::testRunner()
 {
-    Debugger::AnalyzerConnection connection;
-    LocalQmlProfilerRunner::Configuration configuration;
     configuration.debuggee.executable = "\\-/|\\-/";
     configuration.debuggee.environment = Utils::Environment::systemEnvironment();
 
     // should not be used anywhere but cannot be empty
     configuration.socket = connection.analyzerSocket = QString("invalid");
 
-    Debugger::AnalyzerRunControl *rc = Debugger::createAnalyzerRunControl(
+    rc = Debugger::createAnalyzerRunControl(
                 nullptr, ProjectExplorer::Constants::QML_PROFILER_RUN_MODE);
     rc->setConnection(connection);
     auto runner = new LocalQmlProfilerRunner(configuration, rc);
+    connectRunner(runner);
 
-    bool running = false;
+    rc->initiateStart();
-    int runCount = 0;
+    QTimer::singleShot(0, this, &LocalQmlProfilerRunnerTest::testRunner1);
-
+}
-    auto connectRunner = [&]() {
-        connect(runner, &LocalQmlProfilerRunner::started, this, [&running, &runCount](){
-            QVERIFY(!running);
-            ++runCount;
-            running = true;
-        });
-        connect(runner, &LocalQmlProfilerRunner::stopped, this, [&running](){
-            QVERIFY(running);
-            running = false;
-        });
-    };
-
-    connectRunner();
-
-    rc->start();
 
+void LocalQmlProfilerRunnerTest::testRunner1()
+{
     QTRY_COMPARE_WITH_TIMEOUT(runCount, 1, 10000);
     QTRY_VERIFY_WITH_TIMEOUT(!running, 10000);
 
@@ -84,10 +83,14 @@ void LocalQmlProfilerRunnerTest::testRunner()
     rc = Debugger::createAnalyzerRunControl(
                 nullptr, ProjectExplorer::Constants::QML_PROFILER_RUN_MODE);
     rc->setConnection(connection);
-    runner = new LocalQmlProfilerRunner(configuration, rc);
+    auto runner = new LocalQmlProfilerRunner(configuration, rc);
-    connectRunner();
+    connectRunner(runner);
-    rc->start();
+    rc->initiateStart();
+    QTimer::singleShot(0, this, &LocalQmlProfilerRunnerTest::testRunner2);
+}
 
+void LocalQmlProfilerRunnerTest::testRunner2()
+{
     QTRY_COMPARE_WITH_TIMEOUT(runCount, 2, 10000);
     QTRY_VERIFY_WITH_TIMEOUT(!running, 10000);
 
@@ -101,17 +104,25 @@ void LocalQmlProfilerRunnerTest::testRunner()
     rc = Debugger::createAnalyzerRunControl(
                 nullptr, ProjectExplorer::Constants::QML_PROFILER_RUN_MODE);
     rc->setConnection(connection);
-    runner = new LocalQmlProfilerRunner(configuration, rc);
+    auto runner = new LocalQmlProfilerRunner(configuration, rc);
-    connectRunner();
+    connectRunner(runner);
-    rc->start();
+    rc->initiateStart();
 
-    QTRY_COMPARE_WITH_TIMEOUT(runCount, 3, 10000);
+    QTimer::singleShot(0, this, &LocalQmlProfilerRunnerTest::testRunner3);
-    rc->stop();
-    QTRY_VERIFY_WITH_TIMEOUT(!running, 10000);
-
-    delete rc;
 }
 
+void LocalQmlProfilerRunnerTest::testRunner3()
+{
+    QTRY_COMPARE_WITH_TIMEOUT(runCount, 3, 10000);
+    rc->initiateStop();
+    QTimer::singleShot(0, this, &LocalQmlProfilerRunnerTest::testRunner4);
+}
+
+void LocalQmlProfilerRunnerTest::testRunner4()
+{
+    QTRY_VERIFY_WITH_TIMEOUT(!running, 10000);
+    delete rc;
+}
 
 void LocalQmlProfilerRunnerTest::testFindFreePort()
 {

src/plugins/qmlprofiler/tests/localqmlprofilerrunner_test.h

@@ -27,7 +27,7 @@
 
 #include <qmlprofiler/localqmlprofilerrunner.h>
 #include <qmlprofiler/qmlprofilermodelmanager.h>
-#include <QObject>
+#include <debugger/analyzer/analyzerstartparameters.h>
 
 namespace QmlProfiler {
 namespace Internal {
@@ -42,6 +42,19 @@ private slots:
     void testRunner();
     void testFindFreePort();
     void testFindFreeSocket();
+
+private:
+    void connectRunner(LocalQmlProfilerRunner *runner);
+    void testRunner1();
+    void testRunner2();
+    void testRunner3();
+    void testRunner4();
+
+    bool running = false;
+    int runCount = 0;
+    Debugger::AnalyzerRunControl *rc = nullptr;
+    Debugger::AnalyzerConnection connection;
+    LocalQmlProfilerRunner::Configuration configuration;
 };
 
 } // namespace Internal

src/plugins/qnx/qnxruncontrol.cpp

@@ -57,10 +57,10 @@ QnxRunControl::QnxRunControl(RunConfiguration *runConfig)
         connect(m_slog2Info, &Slog2InfoRunner::commandMissing, this, &QnxRunControl::printMissingWarning);
 }
 
-RunControl::StopResult QnxRunControl::stop()
+void QnxRunControl::stop()
 {
     m_slog2Info->stop();
-    return SimpleRunControl::stop();
+    SimpleRunControl::stop();
 }
 
 void QnxRunControl::printMissingWarning()

src/plugins/qnx/qnxruncontrol.h

@@ -38,7 +38,7 @@ class QnxRunControl : public ProjectExplorer::SimpleRunControl
 public:
     explicit QnxRunControl(ProjectExplorer::RunConfiguration *runConfig);
 
-    StopResult stop() override;
+    void stop() override;
 
 private:
     void printMissingWarning();

src/plugins/valgrind/memcheckengine.cpp

@@ -81,11 +81,11 @@ void MemcheckRunControl::start()
     ValgrindRunControl::start();
 }
 
-RunControl::StopResult MemcheckRunControl::stop()
+void MemcheckRunControl::stop()
 {
     disconnect(&m_parser, &ThreadedParser::internalError,
                this, &MemcheckRunControl::internalParserError);
-    return ValgrindRunControl::stop();
+    ValgrindRunControl::stop();
 }
 
 QStringList MemcheckRunControl::toolArguments() const
@@ -163,7 +163,7 @@ void MemcheckWithGdbRunControl::startDebugger()
     QTC_ASSERT(gdbRunControl, return);
     connect(gdbRunControl, &RunControl::finished,
             gdbRunControl, &RunControl::deleteLater);
-    gdbRunControl->start();
+    gdbRunControl->initiateStart();
 }
 
 void MemcheckWithGdbRunControl::appendLog(const QByteArray &data)

src/plugins/valgrind/memcheckengine.h

@@ -43,7 +43,7 @@ public:
                        Core::Id runMode);
 
     void start() override;
-    StopResult stop() override;
+    void stop() override;
 
     QStringList suppressionFiles() const;
 

src/plugins/valgrind/valgrindengine.cpp

@@ -107,11 +107,10 @@ void ValgrindRunControl::start()
     }
 }
 
-RunControl::StopResult ValgrindRunControl::stop()
+void ValgrindRunControl::stop()
 {
     m_isStopping = true;
     runner()->stop();
-    return AsynchronousStop;
 }
 
 QString ValgrindRunControl::executable() const

src/plugins/valgrind/valgrindengine.h

@@ -46,7 +46,7 @@ public:
                        Core::Id runMode);
 
     void start() override;
-    StopResult stop() override;
+    void stop() override;
     bool supportsReRunning() const override { return false; }
 
     QString executable() const;

src/plugins/winrt/winrtruncontrol.cpp

@@ -67,13 +67,12 @@ void WinRtRunControl::start()
         reportApplicationStart();
 }
 
-RunControl::StopResult WinRtRunControl::stop()
+void WinRtRunControl::stop()
 {
     if (m_state == StoppedState)
-        return StoppedSynchronously;
+        return;
 
     m_runner->stop();
-    return AsynchronousStop;
 }
 
 void WinRtRunControl::onProcessStarted()

src/plugins/winrt/winrtruncontrol.h

@@ -51,7 +51,7 @@ public:
     explicit WinRtRunControl(WinRtRunConfiguration *runConfiguration, Core::Id mode);
 
     void start() override;
-    StopResult stop() override;
+    void stop() override;
 
 private:
     enum State { StartingState, StartedState, StoppedState };