From f103f283c2c2d488877e7159b4b8b8f8d3ae1976 Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Wed, 16 Apr 2014 10:27:59 +0200 Subject: 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 --- src/lib/corelib/buildgraph/executor.cpp | 4 +++- src/lib/corelib/buildgraph/jscommandexecutor.cpp | 4 +--- src/lib/corelib/language/loader.cpp | 13 +++++++++++++ src/lib/corelib/language/loader.h | 7 ++++++- src/lib/corelib/language/scriptengine.cpp | 12 ++++++++++++ src/lib/corelib/language/scriptengine.h | 5 +++++ .../api/testdata/infinite-loop-resolving/project.qbs | 5 +++++ tests/auto/api/tst_api.cpp | 18 ++++++++++++++++-- tests/auto/api/tst_api.h | 5 +++-- 9 files changed, 64 insertions(+), 9 deletions(-) create mode 100644 tests/auto/api/testdata/infinite-loop-resolving/project.qbs 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 #include +#include namespace qbs { namespace Internal { @@ -105,6 +106,8 @@ TopLevelProjectPtr Loader::loadProject(const SetupProjectParameters ¶meters) .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 ¶meters) 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 ¶meters) 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 +#include #include 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 ¶meters); +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 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("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 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(); -- cgit v1.2.3