ProjectExplorer: Sanitize AbstractProcessStep cancellation

This build step runs in the main thread and thus can and should use the
cancel() function, rather than employing a polling approach.

Change-Id: Iba013f474db79820632e18224ad485bc1c4ec9b5
Reviewed-by: hjk <hjk@qt.io>
This commit is contained in:
Christian Kandeler
2019-01-28 09:59:24 +01:00
parent 719a51a03d
commit 7a5198e867
2 changed files with 6 additions and 17 deletions

View File

@@ -104,7 +104,6 @@ public:
Private(AbstractProcessStep *q) : q(q) {} Private(AbstractProcessStep *q) : q(q) {}
AbstractProcessStep *q; AbstractProcessStep *q;
QTimer m_timer;
QFutureInterface<bool> *m_futureInterface = nullptr; QFutureInterface<bool> *m_futureInterface = nullptr;
std::unique_ptr<Utils::QtcProcess> m_process; std::unique_ptr<Utils::QtcProcess> m_process;
std::unique_ptr<IOutputParser> m_outputParserChain; std::unique_ptr<IOutputParser> m_outputParserChain;
@@ -126,8 +125,6 @@ AbstractProcessStep::AbstractProcessStep(BuildStepList *bsl, Core::Id id) :
BuildStep(bsl, id), BuildStep(bsl, id),
d(new Private(this)) d(new Private(this))
{ {
d->m_timer.setInterval(500);
connect(&d->m_timer, &QTimer::timeout, this, &AbstractProcessStep::checkForCancel);
setRunInGuiThread(true); setRunInGuiThread(true);
} }
@@ -256,7 +253,11 @@ void AbstractProcessStep::run(QFutureInterface<bool> &fi)
return; return;
} }
processStarted(); processStarted();
d->m_timer.start(); }
void AbstractProcessStep::cancel()
{
Core::Reaper::reap(d->m_process.release());
} }
ProcessParameters *AbstractProcessStep::processParameters() ProcessParameters *AbstractProcessStep::processParameters()
@@ -328,7 +329,6 @@ void AbstractProcessStep::processStartupFailed()
.arg(QDir::toNativeSeparators(d->m_param.effectiveCommand()), .arg(QDir::toNativeSeparators(d->m_param.effectiveCommand()),
d->m_param.prettyArguments()), d->m_param.prettyArguments()),
BuildStep::OutputFormat::ErrorMessage); BuildStep::OutputFormat::ErrorMessage);
d->m_timer.stop();
} }
/*! /*!
@@ -425,15 +425,6 @@ QFutureInterface<bool> *AbstractProcessStep::futureInterface() const
return d->m_futureInterface; return d->m_futureInterface;
} }
void AbstractProcessStep::checkForCancel()
{
if (d->m_futureInterface->isCanceled() && d->m_timer.isActive()) {
d->m_timer.stop();
Core::Reaper::reap(d->m_process.release());
}
}
void AbstractProcessStep::taskAdded(const Task &task, int linkedOutputLines, int skipLines) void AbstractProcessStep::taskAdded(const Task &task, int linkedOutputLines, int skipLines)
{ {
// Do not bother to report issues if we do not care about the results of // Do not bother to report issues if we do not care about the results of
@@ -500,8 +491,6 @@ void AbstractProcessStep::outputAdded(const QString &string, BuildStep::OutputFo
void AbstractProcessStep::slotProcessFinished(int, QProcess::ExitStatus) void AbstractProcessStep::slotProcessFinished(int, QProcess::ExitStatus)
{ {
d->m_timer.stop();
QProcess *process = d->m_process.get(); QProcess *process = d->m_process.get();
if (!process) // Happens when the process was canceled and handed over to the Reaper. if (!process) // Happens when the process was canceled and handed over to the Reaper.
process = qobject_cast<QProcess *>(sender()); // The process was canceled! process = qobject_cast<QProcess *>(sender()); // The process was canceled!

View File

@@ -43,6 +43,7 @@ class PROJECTEXPLORER_EXPORT AbstractProcessStep : public BuildStep
public: public:
bool init() override; bool init() override;
void run(QFutureInterface<bool> &) override; void run(QFutureInterface<bool> &) override;
void cancel() override;
ProcessParameters *processParameters(); ProcessParameters *processParameters();
@@ -72,7 +73,6 @@ private:
void processReadyReadStdOutput(); void processReadyReadStdOutput();
void processReadyReadStdError(); void processReadyReadStdError();
void slotProcessFinished(int, QProcess::ExitStatus); void slotProcessFinished(int, QProcess::ExitStatus);
void checkForCancel();
void cleanUp(QProcess *process); void cleanUp(QProcess *process);