From cd5ba755056a70f86fa6b9d7789090f9c84927de Mon Sep 17 00:00:00 2001 From: hjk Date: Thu, 13 Jul 2017 15:24:04 +0200 Subject: [PATCH] ProjectExplorer: Use a real 'finishing' phase for RunControls Instead of a blunt delete() RunControl::initiateFinish() is triggered by the closing of the application output instead. The rampdown process is basically the same as stop() now, except for the other success signal (new finished()) and the final self-destruction of the runcontrol. stop() itself triggers initiateStop() on all running workers in parallel (before it was in the order of start). This gives downstream complex worker combinations the flexibility to use any order it wants by ignoring stop() on 'uninteresting' workers, and centralizing rampdown e.g. in the main worker. That setup should be rare in practice, but seems needed in some profiler cases. Change-Id: I986a152a663754206709ed4df0d4568847afad17 Reviewed-by: Christian Kandeler --- src/plugins/projectexplorer/appoutputpane.cpp | 8 +- src/plugins/projectexplorer/appoutputpane.h | 3 +- .../projectexplorer/runconfiguration.cpp | 180 +++++++++++++++--- .../projectexplorer/runconfiguration.h | 2 + src/plugins/valgrind/memchecktool.cpp | 1 + src/plugins/valgrind/valgrindengine.cpp | 1 - 6 files changed, 162 insertions(+), 33 deletions(-) diff --git a/src/plugins/projectexplorer/appoutputpane.cpp b/src/plugins/projectexplorer/appoutputpane.cpp index 9e0617454bd..1e6ef0efd68 100644 --- a/src/plugins/projectexplorer/appoutputpane.cpp +++ b/src/plugins/projectexplorer/appoutputpane.cpp @@ -420,7 +420,8 @@ void AppOutputPane::createNewOutputWindow(RunControl *rc) if (tabIndex != -1) { RunControlTab &tab = m_runControlTabs[tabIndex]; // Reuse this tab - delete tab.runControl; + if (tab.runControl) + tab.runControl->initiateFinish(); tab.runControl = rc; tab.window->setFormatter(rc ? rc->outputFormatter() : nullptr); @@ -559,7 +560,7 @@ bool AppOutputPane::closeTabs(CloseTabMode mode) QList AppOutputPane::allRunControls() const { return Utils::transform(m_runControlTabs,[](const RunControlTab &tab) { - return tab.runControl; + return tab.runControl.data(); }); } @@ -596,7 +597,8 @@ bool AppOutputPane::closeTab(int tabIndex, CloseTabMode closeTabMode) m_tabWidget->removeTab(tabIndex); delete m_runControlTabs[index].window; - delete m_runControlTabs[index].runControl; + m_runControlTabs[index].runControl->initiateFinish(); // Will self-destruct. + m_runControlTabs[index].runControl = 0; m_runControlTabs.removeAt(index); updateCloseActions(); diff --git a/src/plugins/projectexplorer/appoutputpane.h b/src/plugins/projectexplorer/appoutputpane.h index ba3839ca6af..b260447ba5a 100644 --- a/src/plugins/projectexplorer/appoutputpane.h +++ b/src/plugins/projectexplorer/appoutputpane.h @@ -25,6 +25,7 @@ #pragma once +#include #include #include @@ -124,7 +125,7 @@ private: public: explicit RunControlTab(RunControl *runControl = nullptr, Core::OutputWindow *window = nullptr); - RunControl *runControl; + QPointer runControl; Core::OutputWindow *window; BehaviorOnOutput behaviorOnOutput = Flash; }; diff --git a/src/plugins/projectexplorer/runconfiguration.cpp b/src/plugins/projectexplorer/runconfiguration.cpp index 093c261ad22..8cbfaf7fd21 100644 --- a/src/plugins/projectexplorer/runconfiguration.cpp +++ b/src/plugins/projectexplorer/runconfiguration.cpp @@ -602,7 +602,7 @@ static QString stateName(RunWorkerState s) SN(RunWorkerState::Done) SN(RunWorkerState::Failed) } - return QLatin1String(""); + return QString("").arg(int(s)); # undef SN } @@ -616,7 +616,7 @@ public: RunWorker *q; RunWorkerState state = RunWorkerState::Initialized; - RunControl *runControl; + QPointer runControl; QList dependencies; QString id; @@ -635,6 +635,8 @@ enum class RunControlState Running, // All good and running. Stopping, // initiateStop() was called, stop application/tool Stopped, // all good, but stopped. Can possibly be re-started + Finishing, // Application tab manually closed + Finished // Final state, will self-destruct with deleteLater() }; static QString stateName(RunControlState s) @@ -646,8 +648,10 @@ static QString stateName(RunControlState s) SN(RunControlState::Running) SN(RunControlState::Stopping) SN(RunControlState::Stopped) + SN(RunControlState::Finishing) + SN(RunControlState::Finished) } - return QLatin1String(""); + return QString("").arg(int(s)); # undef SN } @@ -669,8 +673,11 @@ public: ~RunControlPrivate() { - QTC_CHECK(state == RunControlState::Stopped || state == RunControlState::Initialized); + QTC_CHECK(state == RunControlState::Finished || state == RunControlState::Initialized); + disconnect(); + q = nullptr; qDeleteAll(m_workers); + m_workers.clear(); delete outputFormatter; } @@ -685,7 +692,7 @@ public: void initiateReStart(); void continueStart(); void initiateStop(); - void continueStop(); + void initiateFinish(); void onWorkerStarted(RunWorker *worker); void onWorkerStopped(RunWorker *worker); @@ -704,7 +711,7 @@ public: Utils::Icon icon; const QPointer runConfiguration; // Not owned. QPointer project; // Not owned. - Utils::OutputFormatter *outputFormatter = nullptr; + QPointer outputFormatter = nullptr; std::function promptToStop; std::vector m_factories; @@ -769,6 +776,11 @@ void RunControl::initiateStop() d->initiateStop(); } +void RunControl::initiateFinish() +{ + d->initiateFinish(); +} + using WorkerCreators = QHash; static WorkerCreators &theWorkerCreators() @@ -890,15 +902,8 @@ void RunControlPrivate::initiateStop() { checkState(RunControlState::Running); setState(RunControlState::Stopping); - debugMessage("Queue: Stopping"); + debugMessage("Queue: Stopping for all workers"); - continueStop(); -} - -void RunControlPrivate::continueStop() -{ - debugMessage("Continue Stopping"); - checkState(RunControlState::Stopping); bool allDone = true; for (RunWorker *worker : m_workers) { if (worker) { @@ -914,17 +919,17 @@ void RunControlPrivate::continueStop() allDone = false; break; case RunWorkerState::Starting: - worker->d->state = RunWorkerState::Stopping; debugMessage(" " + workerId + " was Starting, queuing stop"); - allDone = false; + worker->d->state = RunWorkerState::Stopping; QTimer::singleShot(0, worker, &RunWorker::initiateStop); - return; // Sic. + allDone = false; + break; case RunWorkerState::Running: debugMessage(" " + workerId + " was Running, queuing stop"); worker->d->state = RunWorkerState::Stopping; allDone = false; QTimer::singleShot(0, worker, &RunWorker::initiateStop); - return; // Sic. + break; case RunWorkerState::Done: debugMessage(" " + workerId + " was Done. Good."); break; @@ -937,8 +942,59 @@ void RunControlPrivate::continueStop() } } if (allDone) { - debugMessage("All workers stopped. Set runControl to Stopped"); + debugMessage("All stopped."); setState(RunControlState::Stopped); + } else { + debugMessage("Not all workers stopped. Waiting..."); + } +} + +void RunControlPrivate::initiateFinish() +{ + setState(RunControlState::Finishing); + debugMessage("Ramping down"); + + bool allDone = true; + for (RunWorker *worker : m_workers) { + if (worker) { + const QString &workerId = worker->d->id; + debugMessage(" Examining worker " + workerId); + switch (worker->d->state) { + case RunWorkerState::Initialized: + debugMessage(" " + workerId + " was Initialized, setting to Done"); + worker->d->state = RunWorkerState::Done; + break; + case RunWorkerState::Stopping: + debugMessage(" " + workerId + " was already Stopping. Keeping it that way"); + allDone = false; + break; + case RunWorkerState::Starting: + debugMessage(" " + workerId + " was Starting, queuing stop"); + worker->d->state = RunWorkerState::Stopping; + QTimer::singleShot(0, worker, &RunWorker::initiateStop); + allDone = false; + break; + case RunWorkerState::Running: + debugMessage(" " + workerId + " was Running, queuing stop"); + worker->d->state = RunWorkerState::Stopping; + allDone = false; + QTimer::singleShot(0, worker, &RunWorker::initiateStop); + break; + case RunWorkerState::Done: + debugMessage(" " + workerId + " was Done. Good."); + break; + case RunWorkerState::Failed: + debugMessage(" " + workerId + " was Failed. Good"); + break; + } + } else { + debugMessage("Found unknown deleted worker"); + } + } + if (allDone) { + setState(RunControlState::Finished); + } else { + debugMessage("Not all workers finished. Waiting..."); } } @@ -954,7 +1010,6 @@ void RunControlPrivate::onWorkerStarted(RunWorker *worker) showError(tr("Unexpected run control state %1 when worker %2 started") .arg(stateName(state)) .arg(worker->d->id)); - //setState(RunControlState::Stopped); } void RunControlPrivate::onWorkerFailed(RunWorker *worker, const QString &msg) @@ -978,13 +1033,72 @@ void RunControlPrivate::onWorkerStopped(RunWorker *worker) worker->d->state = RunWorkerState::Done; debugMessage(workerId + " stopped expectedly."); break; + case RunWorkerState::Done: + worker->d->state = RunWorkerState::Done; + debugMessage(workerId + " stopped twice. Huh? But harmless."); + return; // Sic! default: debugMessage(workerId + " stopped unexpectedly in state" + stateName(worker->d->state)); worker->d->state = RunWorkerState::Failed; break; } - continueStop(); + + debugMessage("Checking whether all stopped"); + bool allDone = true; + for (RunWorker *worker : m_workers) { + if (worker) { + const QString &workerId = worker->d->id; + debugMessage(" Examining worker " + workerId); + switch (worker->d->state) { + case RunWorkerState::Initialized: + debugMessage(" " + workerId + " was Initialized, setting to Done"); + worker->d->state = RunWorkerState::Done; + break; + case RunWorkerState::Starting: + worker->d->state = RunWorkerState::Stopping; + debugMessage(" " + workerId + " was Starting, queuing stop"); + allDone = false; + break; + case RunWorkerState::Running: + debugMessage(" " + workerId + " was Running, queuing stop"); + worker->d->state = RunWorkerState::Stopping; + allDone = false; + break; + case RunWorkerState::Stopping: + debugMessage(" " + workerId + " was already Stopping. Keeping it that way"); + allDone = false; + break; + case RunWorkerState::Done: + debugMessage(" " + workerId + " was Done. Good."); + break; + case RunWorkerState::Failed: + debugMessage(" " + workerId + " was Failed. Good"); + break; + } + } else { + debugMessage("Found unknown deleted worker"); + } + } + if (state == RunControlState::Finishing) { + if (allDone) { + debugMessage("All finished. Deleting myself"); + setState(RunControlState::Finished); + } else { + debugMessage("Not all workers finished. Waiting..."); + } + } else { + if (allDone) { + if (state == RunControlState::Stopped) { + debugMessage("All workers stopped, but runControl was already stopped."); + } else { + debugMessage("All workers stopped. Set runControl to Stopped"); + setState(RunControlState::Stopped); + } + } else { + debugMessage("Not all workers stopped. Waiting..."); + } + } } void RunControlPrivate::showError(const QString &msg) @@ -1185,15 +1299,23 @@ bool RunControlPrivate::isAllowedTransition(RunControlState from, RunControlStat { switch (from) { case RunControlState::Initialized: - return to == RunControlState::Starting; + return to == RunControlState::Starting + || to == RunControlState::Finishing; case RunControlState::Starting: - return to == RunControlState::Running; + return to == RunControlState::Running + || to == RunControlState::Finishing; case RunControlState::Running: return to == RunControlState::Stopping - || to == RunControlState::Stopped; + || to == RunControlState::Stopped + || to == RunControlState::Finishing; case RunControlState::Stopping: - return to == RunControlState::Stopped; + return to == RunControlState::Stopped + || to == RunControlState::Finishing; case RunControlState::Stopped: + return to == RunControlState::Finishing; + case RunControlState::Finishing: + return to == RunControlState::Finished; + case RunControlState::Finished: return false; } return false; @@ -1223,11 +1345,13 @@ void RunControlPrivate::setState(RunControlState newState) break; case RunControlState::Stopped: q->setApplicationProcessHandle(Utils::ProcessHandle()); - foreach (auto worker, m_workers) - if (worker) - worker->onFinished(); emit q->stopped(); break; + case RunControlState::Finished: + emit q->finished(); + debugMessage("All finished. Deleting myself"); + deleteLater(); + break; default: break; } diff --git a/src/plugins/projectexplorer/runconfiguration.h b/src/plugins/projectexplorer/runconfiguration.h index 264ec4fd50a..5a8fe5dbdc6 100644 --- a/src/plugins/projectexplorer/runconfiguration.h +++ b/src/plugins/projectexplorer/runconfiguration.h @@ -397,6 +397,7 @@ public: void initiateStart(); void initiateReStart(); void initiateStop(); + void initiateFinish(); bool promptToStop(bool *optionalPrompt = nullptr) const; void setPromptToStop(const std::function &promptToStop); @@ -480,6 +481,7 @@ signals: void aboutToStart(); void started(); void stopped(); + void finished(); void applicationProcessHandleChanged(QPrivateSignal); // Use setApplicationProcessHandle private: diff --git a/src/plugins/valgrind/memchecktool.cpp b/src/plugins/valgrind/memchecktool.cpp index a69e2eac606..cab6dea44c9 100644 --- a/src/plugins/valgrind/memchecktool.cpp +++ b/src/plugins/valgrind/memchecktool.cpp @@ -562,6 +562,7 @@ RunWorker *MemcheckTool::createRunWorker(RunControl *runControl) connect(runTool, &MemcheckToolRunner::internalParserError, this, &MemcheckTool::internalParserError); connect(runTool, &MemcheckToolRunner::stopped, this, &MemcheckTool::engineFinished); + m_stopAction->disconnect(); connect(m_stopAction, &QAction::triggered, runControl, &RunControl::initiateStop); m_toolBusy = true; diff --git a/src/plugins/valgrind/valgrindengine.cpp b/src/plugins/valgrind/valgrindengine.cpp index 3cb1a4245b2..13beca1fa06 100644 --- a/src/plugins/valgrind/valgrindengine.cpp +++ b/src/plugins/valgrind/valgrindengine.cpp @@ -106,7 +106,6 @@ void ValgrindToolRunner::stop() { m_isStopping = true; m_runner.stop(); - reportStopped(); // FIXME: Restrict to non-running scenarios? } QString ValgrindToolRunner::executable() const