Allow long-running commands to be canceled.

At the moment, canceling a build waits for the current
command to finish, which means that a badly behaving process
or piece of JavaScript code can block qbs indefinitely.

Task-number: QBS-552
Change-Id: I8ac23f068dd6083905a9681097da6b970c0b646b
Reviewed-by: Joerg Bornemann <joerg.bornemann@digia.com>

10 files changed, 53 insertions, 22 deletions

diff --git a/src/lib/corelib/buildgraph/abstractcommandexecutor.h b/src/lib/corelib/buildgraph/abstractcommandexecutor.h
index a4f781d62..5110a9ee8 100644
--- a/src/lib/corelib/buildgraph/abstractcommandexecutor.h
+++ b/src/lib/corelib/buildgraph/abstractcommandexecutor.h
@@ -52,7 +52,7 @@ public:
     void setMainThreadScriptEngine(ScriptEngine *engine) { m_mainThreadScriptEngine = engine; }
     void setDryRunEnabled(bool enabled) { m_dryRun = enabled; }
 
-    virtual void waitForFinished() = 0;
+    virtual void cancel() = 0;
 
 public slots:
     void start(Transformer *transformer, const AbstractCommand *cmd);
diff --git a/src/lib/corelib/buildgraph/executor.cpp b/src/lib/corelib/buildgraph/executor.cpp
index aab2c4220..1e5a66feb 100644
--- a/src/lib/corelib/buildgraph/executor.cpp
+++ b/src/lib/corelib/buildgraph/executor.cpp
@@ -69,10 +69,14 @@ Executor::Executor(const Logger &logger, QObject *parent)
     , m_logger(logger)
     , m_progressObserver(0)
     , m_state(ExecutorIdle)
+    , m_cancelationTimer(new QTimer(this))
     , m_doTrace(logger.traceEnabled())
     , m_doDebug(logger.debugEnabled())
 {
     m_inputArtifactScanContext = new InputArtifactScannerContext(&m_scanResultCache);
+    m_cancelationTimer->setSingleShot(false);
+    m_cancelationTimer->setInterval(1000);
+    connect(m_cancelationTimer, SIGNAL(timeout()), SLOT(checkForCancellation()));
 }
 
 Executor::~Executor()
@@ -218,6 +222,8 @@ void Executor::doBuild()
         m_logger.qbsTrace() << "Nothing to do at all, finishing.";
         QTimer::singleShot(0, this, SLOT(finish())); // Don't call back on the caller.
     }
+    if (m_progressObserver)
+        m_cancelationTimer->start();
 }
 
 void Executor::setBuildOptions(const BuildOptions &buildOptions)
@@ -861,11 +867,20 @@ void Executor::finish()
     if (m_explicitlyCanceled)
         m_error.append(Tr::tr("Build canceled%1.").arg(configString()));
     setState(ExecutorIdle);
-    if (m_progressObserver)
+    if (m_progressObserver) {
         m_progressObserver->setFinished();
+        m_cancelationTimer->stop();
+    }
     emit finished();
 }
 
+void Executor::checkForCancellation()
+{
+    QBS_ASSERT(m_progressObserver, return);
+    if (m_state == ExecutorRunning && m_progressObserver->canceled())
+        cancelJobs();
+}
+
 bool Executor::visit(Artifact *artifact)
 {
     buildArtifact(artifact);
diff --git a/src/lib/corelib/buildgraph/executor.h b/src/lib/corelib/buildgraph/executor.h
index ba5011ce0..fc59fc04b 100644
--- a/src/lib/corelib/buildgraph/executor.h
+++ b/src/lib/corelib/buildgraph/executor.h
@@ -43,6 +43,10 @@
 #include <QObject>
 #include <queue>
 
+QT_BEGIN_NAMESPACE
+class QTimer;
+QT_END_NAMESPACE
+
 namespace qbs {
 class ProcessResult;
 
@@ -80,6 +84,7 @@ signals:
 private slots:
     void onJobFinished(const qbs::ErrorInfo &err);
     void finish();
+    void checkForCancellation();
 
 private:
     // BuildGraphVisitor implementation
@@ -150,6 +155,7 @@ private:
     ErrorInfo m_error;
     bool m_explicitlyCanceled;
     FileTags m_activeFileTags;
+    QTimer * const m_cancelationTimer;
     const bool m_doTrace;
     const bool m_doDebug;
 };
diff --git a/src/lib/corelib/buildgraph/executorjob.cpp b/src/lib/corelib/buildgraph/executorjob.cpp
index 1ad82cdb5..e2ed11ba2 100644
--- a/src/lib/corelib/buildgraph/executorjob.cpp
+++ b/src/lib/corelib/buildgraph/executorjob.cpp
@@ -96,18 +96,9 @@ void ExecutorJob::run(Transformer *t)
 
 void ExecutorJob::cancel()
 {
-    if (!m_transformer)
-        return;
-    if (m_currentCommandIdx < m_transformer->commands.count() - 1) {
-        m_error = ErrorInfo(tr("Transformer execution canceled."));
-        m_currentCommandIdx = m_transformer->commands.count();
-    }
-}
-
-void ExecutorJob::waitForFinished()
-{
-    if (m_currentCommandExecutor)
-        m_currentCommandExecutor->waitForFinished();
+    QBS_ASSERT(m_currentCommandExecutor, return);
+    m_error = ErrorInfo(tr("Transformer execution canceled."));
+    m_currentCommandExecutor->cancel();
 }
 
 void ExecutorJob::runNextCommand()
@@ -137,7 +128,9 @@ void ExecutorJob::runNextCommand()
 void ExecutorJob::onCommandFinished(const ErrorInfo &err)
 {
     QBS_ASSERT(m_transformer, return);
-    if (err.hasError()) {
+    if (m_error.hasError()) { // Canceled?
+        setFinished();
+    } else if (err.hasError()) {
         m_error = err;
         setFinished();
     } else {
diff --git a/src/lib/corelib/buildgraph/executorjob.h b/src/lib/corelib/buildgraph/executorjob.h
index a6e4d1674..27be8120e 100644
--- a/src/lib/corelib/buildgraph/executorjob.h
+++ b/src/lib/corelib/buildgraph/executorjob.h
@@ -59,7 +59,6 @@ public:
     void setDryRun(bool enabled);
     void run(Transformer *t);
     void cancel();
-    void waitForFinished();
 
 signals:
     void reportCommandDescription(const QString &highlight, const QString &message);
diff --git a/src/lib/corelib/buildgraph/jscommandexecutor.cpp b/src/lib/corelib/buildgraph/jscommandexecutor.cpp
index 698002988..0b65c813d 100644
--- a/src/lib/corelib/buildgraph/jscommandexecutor.cpp
+++ b/src/lib/corelib/buildgraph/jscommandexecutor.cpp
@@ -42,6 +42,7 @@
 #include <tools/error.h>
 
 #include <QEventLoop>
+#include <QMetaObject>
 #include <QThread>
 #include <QTimer>
 
@@ -70,6 +71,12 @@ public:
         return m_result;
     }
 
+    Q_INVOKABLE void cancel()
+    {
+        QBS_ASSERT(m_scriptEngine, return);
+        m_scriptEngine->abortEvaluation();
+    }
+
 signals:
     void finished();
 
@@ -111,8 +118,10 @@ public slots:
 private:
     ScriptEngine *provideScriptEngine()
     {
-        if (!m_scriptEngine)
+        if (!m_scriptEngine) {
             m_scriptEngine = new ScriptEngine(m_logger, this);
+            m_scriptEngine->setProcessEventsInterval(1000); // So long-running code can be aborted.
+        }
         return m_scriptEngine;
     }
 
@@ -165,6 +174,11 @@ void JsCommandExecutor::doStart()
     emit startRequested(jsCommand(), transformer());
 }
 
+void JsCommandExecutor::cancel()
+{
+    QMetaObject::invokeMethod(m_objectInThread, "cancel", Qt::QueuedConnection);
+}
+
 void JsCommandExecutor::onJavaScriptCommandFinished()
 {
     m_running = false;
diff --git a/src/lib/corelib/buildgraph/jscommandexecutor.h b/src/lib/corelib/buildgraph/jscommandexecutor.h
index 94a25e610..ece752918 100644
--- a/src/lib/corelib/buildgraph/jscommandexecutor.h
+++ b/src/lib/corelib/buildgraph/jscommandexecutor.h
@@ -56,6 +56,8 @@ private slots:
 
 private:
     void doStart();
+    void cancel();
+
     void waitForFinished();
 
     const JavaScriptCommand *jsCommand() const;
diff --git a/src/lib/corelib/buildgraph/processcommandexecutor.cpp b/src/lib/corelib/buildgraph/processcommandexecutor.cpp
index e1733ec40..cb1d9604a 100644
--- a/src/lib/corelib/buildgraph/processcommandexecutor.cpp
+++ b/src/lib/corelib/buildgraph/processcommandexecutor.cpp
@@ -159,9 +159,11 @@ void ProcessCommandExecutor::doStart()
     m_arguments = arguments;
 }
 
-void ProcessCommandExecutor::waitForFinished()
+void ProcessCommandExecutor::cancel()
 {
-    m_process.waitForFinished(-1);
+    m_process.terminate();
+    if (!m_process.waitForFinished(1000))
+        m_process.kill();
 }
 
 QString ProcessCommandExecutor::filterProcessOutput(const QByteArray &_output,
diff --git a/src/lib/corelib/buildgraph/processcommandexecutor.h b/src/lib/corelib/buildgraph/processcommandexecutor.h
index c90141303..e93680796 100644
--- a/src/lib/corelib/buildgraph/processcommandexecutor.h
+++ b/src/lib/corelib/buildgraph/processcommandexecutor.h
@@ -61,8 +61,9 @@ private slots:
 
 private:
     void doStart();
-    void waitForFinished();
+    void cancel();
 
+    void waitForFinished();
     void startProcessCommand();
     QString filterProcessOutput(const QByteArray &output, const QString &filterFunctionSource);
     void sendProcessOutput(bool success);
diff --git a/tests/auto/api/tst_api.cpp b/tests/auto/api/tst_api.cpp
index aaafd49ce..13af57c4a 100644
--- a/tests/auto/api/tst_api.cpp
+++ b/tests/auto/api/tst_api.cpp
@@ -442,8 +442,7 @@ void TestApi::infiniteLoop()
     QVERIFY2(!setupJob->error().hasError(), qPrintable(setupJob->error().toString()));
     qbs::Project project = setupJob->project();
     const QScopedPointer<qbs::BuildJob> buildJob(project.buildAllProducts(qbs::BuildOptions()));
-    QTimer::singleShot(1000, setupJob.data(), SLOT(cancel()));
-    QEXPECT_FAIL(0, "QBS-552", Continue);
+    QTimer::singleShot(1000, buildJob.data(), SLOT(cancel()));
     QVERIFY(waitForFinished(buildJob.data(), 3000));
 }