Prevent user code from hanging qbs during resolving.

Long-running commands are handled already, but badly written project
files could still hang qbs with e.g. infinite JS loops on the right
hand side of a binding. Such code can now also be interrupted.

Change-Id: Ie0d114bd37d540e764d5ec5bb323c91bfd64a67a
Reviewed-by: Joerg Bornemann <joerg.bornemann@digia.com>

9 files changed, 64 insertions, 9 deletions

diff --git a/src/lib/corelib/buildgraph/executor.cpp b/src/lib/corelib/buildgraph/executor.cpp
index 1e5a66feb..1b37c17d1 100644
--- a/src/lib/corelib/buildgraph/executor.cpp
+++ b/src/lib/corelib/buildgraph/executor.cpp
@@ -877,8 +877,10 @@ void Executor::finish()
 void Executor::checkForCancellation()
 {
     QBS_ASSERT(m_progressObserver, return);
-    if (m_state == ExecutorRunning && m_progressObserver->canceled())
+    if (m_state == ExecutorRunning && m_progressObserver->canceled()) {
         cancelJobs();
+        m_evalContext->engine()->cancel();
+    }
 }
 
 bool Executor::visit(Artifact *artifact)
diff --git a/src/lib/corelib/buildgraph/jscommandexecutor.cpp b/src/lib/corelib/buildgraph/jscommandexecutor.cpp
index 0b65c813d..e2d1e14e4 100644
--- a/src/lib/corelib/buildgraph/jscommandexecutor.cpp
+++ b/src/lib/corelib/buildgraph/jscommandexecutor.cpp
@@ -118,10 +118,8 @@ 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;
     }
 
diff --git a/src/lib/corelib/language/loader.cpp b/src/lib/corelib/language/loader.cpp
index baf30c474..62c1aadde 100644
--- a/src/lib/corelib/language/loader.cpp
+++ b/src/lib/corelib/language/loader.cpp
@@ -40,6 +40,7 @@
 #include <tools/setupprojectparameters.h>
 
 #include <QDir>
+#include <QTimer>
 
 namespace qbs {
 namespace Internal {
@@ -105,6 +106,8 @@ TopLevelProjectPtr Loader::loadProject(const SetupProjectParameters &parameters)
                     .toMap()));
     m_engine->clearExceptions();
 
+    QTimer cancelationTimer;
+
     // At this point, we cannot set a sensible total effort, because we know nothing about
     // the project yet. That's why we use a placeholder here, so the user at least
     // sees that an operation is starting. The real total effort will be set later when
@@ -112,6 +115,9 @@ TopLevelProjectPtr Loader::loadProject(const SetupProjectParameters &parameters)
     if (m_progressObserver) {
         m_progressObserver->initialize(Tr::tr("Resolving project for configuration %1")
                 .arg(TopLevelProject::deriveId(parameters.buildConfigurationTree())), 1);
+        cancelationTimer.setSingleShot(false);
+        QObject::connect(&cancelationTimer, SIGNAL(timeout()), SLOT(checkForCancelation()));
+        cancelationTimer.start(1000);
     }
 
     ModuleLoaderResult loadResult
@@ -128,5 +134,12 @@ TopLevelProjectPtr Loader::loadProject(const SetupProjectParameters &parameters)
     return project;
 }
 
+void Loader::checkForCancelation()
+{
+    QBS_ASSERT(m_progressObserver, return);
+    if (m_progressObserver->canceled())
+        m_engine->cancel();
+}
+
 } // namespace Internal
 } // namespace qbs
diff --git a/src/lib/corelib/language/loader.h b/src/lib/corelib/language/loader.h
index 5de3302c1..6e2d65817 100644
--- a/src/lib/corelib/language/loader.h
+++ b/src/lib/corelib/language/loader.h
@@ -32,6 +32,7 @@
 #include "forward_decls.h"
 #include <logging/logger.h>
 
+#include <QObject>
 #include <QStringList>
 
 namespace qbs {
@@ -45,8 +46,9 @@ class ProgressObserver;
 class ScriptEngine;
 class ProjectResolver;
 
-class Loader
+class Loader : public QObject
 {
+    Q_OBJECT
 public:
     Loader(ScriptEngine *engine, const Logger &logger);
     ~Loader();
@@ -55,6 +57,9 @@ public:
     void setSearchPaths(const QStringList &searchPaths);
     TopLevelProjectPtr loadProject(const SetupProjectParameters &parameters);
 
+private slots:
+    void checkForCancelation();
+
 private:
     Logger m_logger;
     ProgressObserver *m_progressObserver;
diff --git a/src/lib/corelib/language/scriptengine.cpp b/src/lib/corelib/language/scriptengine.cpp
index bbc84d580..7aa6138dd 100644
--- a/src/lib/corelib/language/scriptengine.cpp
+++ b/src/lib/corelib/language/scriptengine.cpp
@@ -79,6 +79,8 @@ uint qHash(const ScriptEngine::PropertyCacheKey &k, uint seed = 0)
 ScriptEngine::ScriptEngine(const Logger &logger, QObject *parent)
     : QScriptEngine(parent), m_logger(logger)
 {
+    setProcessEventsInterval(1000); // For the cancelation mechanism to work.
+    m_cancelationError = currentContext()->throwValue(tr("Execution canceled"));
     QScriptValue objectProto = globalObject().property(QLatin1String("Object"));
     m_definePropertyFunction = objectProto.property(QLatin1String("defineProperty"));
     QBS_ASSERT(m_definePropertyFunction.isFunction(), /* ignore */);
@@ -457,6 +459,16 @@ QScriptValueList ScriptEngine::argumentList(const QStringList &argumentNames,
     return result;
 }
 
+void ScriptEngine::cancel()
+{
+    QMetaObject::invokeMethod(this, "abort", Qt::QueuedConnection);
+}
+
+void ScriptEngine::abort()
+{
+    abortEvaluation(m_cancelationError);
+}
+
 class JSTypeExtender
 {
 public:
diff --git a/src/lib/corelib/language/scriptengine.h b/src/lib/corelib/language/scriptengine.h
index 3d90f7b0c..c56dece01 100644
--- a/src/lib/corelib/language/scriptengine.h
+++ b/src/lib/corelib/language/scriptengine.h
@@ -116,7 +116,11 @@ public:
         return v.isError() || hasUncaughtException();
     }
 
+    void cancel();
+
 private:
+    Q_INVOKABLE void abort();
+
     void extendJavaScriptBuiltins();
     void installFunction(const QString &name, QScriptValue *functionValue, FunctionSignature f);
     void installImportFunctions();
@@ -161,6 +165,7 @@ private:
     QStack<QStringList> m_extensionSearchPathsStack;
     QScriptValue m_loadFileFunction;
     QScriptValue m_loadExtensionFunction;
+    QScriptValue m_cancelationError;
 };
 
 } // namespace Internal
diff --git a/tests/auto/api/testdata/infinite-loop-resolving/project.qbs b/tests/auto/api/testdata/infinite-loop-resolving/project.qbs
new file mode 100644
index 000000000..b690f5041
--- /dev/null
+++ b/tests/auto/api/testdata/infinite-loop-resolving/project.qbs
@@ -0,0 +1,5 @@
+import qbs
+
+Product {
+    type: { while (true); return "Haha!"; }
+}
diff --git a/tests/auto/api/tst_api.cpp b/tests/auto/api/tst_api.cpp
index 13af57c4a..2e441d220 100644
--- a/tests/auto/api/tst_api.cpp
+++ b/tests/auto/api/tst_api.cpp
@@ -429,7 +429,7 @@ void TestApi::fileTagsFilterOverride()
     QVERIFY(installableFiles.first().targetDirectory().contains("habicht"));
 }
 
-void TestApi::infiniteLoop()
+void TestApi::infiniteLoopBuilding()
 {
     QFETCH(QString, projectDirName);
     qbs::SetupProjectParameters setupParams = defaultSetupParameters();
@@ -446,13 +446,27 @@ void TestApi::infiniteLoop()
     QVERIFY(waitForFinished(buildJob.data(), 3000));
 }
 
-void TestApi::infiniteLoop_data()
+void TestApi::infiniteLoopBuilding_data()
 {
     QTest::addColumn<QString>("projectDirName");
     QTest::newRow("JS Command") << QString("infinite-loop-js");
     QTest::newRow("Process Command") << QString("infinite-loop-process");
 }
 
+void TestApi::infiniteLoopResolving()
+{
+    qbs::SetupProjectParameters setupParams = defaultSetupParameters();
+    const QString projectDir = QDir::cleanPath(m_workingDataDir + "/infinite-loop-resolving");
+    setupParams.setProjectFilePath(projectDir + "/project.qbs");
+    setupParams.setBuildRoot(projectDir);
+    QScopedPointer<qbs::SetupProjectJob> setupJob(qbs::Project::setupProject(setupParams,
+                                                                             m_logSink, 0));
+    QTimer::singleShot(1000, setupJob.data(), SLOT(cancel()));
+    QVERIFY(waitForFinished(setupJob.data(), 3000));
+    QVERIFY2(setupJob->error().toString().toLower().contains("cancel"),
+             qPrintable(setupJob->error().toString()));
+}
+
 void TestApi::installableFiles()
 {
     qbs::SetupProjectParameters setupParams = defaultSetupParameters();
diff --git a/tests/auto/api/tst_api.h b/tests/auto/api/tst_api.h
index 6183346ce..9aab60651 100644
--- a/tests/auto/api/tst_api.h
+++ b/tests/auto/api/tst_api.h
@@ -51,8 +51,9 @@ private slots:
     void changeContent();
     void disabledInstallGroup();
     void fileTagsFilterOverride();
-    void infiniteLoop();
-    void infiniteLoop_data();
+    void infiniteLoopBuilding();
+    void infiniteLoopBuilding_data();
+    void infiniteLoopResolving();
     void installableFiles();
     void listBuildSystemFiles();
     void nonexistingProjectPropertyFromProduct();