remove QtConcurrent dependency from JsCommandExecutor

Fewer dependencies makes bootstrapping easier.
Also, this prevents a potential deadlock on single core machines
when the executor is run through QtConcurrent, like in Qt Creator.

Task-number: QBS-68
Task-number: QBS-307

Change-Id: Id5383a2b5a6bf0d7880f0969e3730c3606e8b1d9
Reviewed-by: Christian Kandeler <christian.kandeler@digia.com>

2 files changed, 71 insertions, 42 deletions

diff --git a/src/lib/buildgraph/jscommandexecutor.cpp b/src/lib/buildgraph/jscommandexecutor.cpp
index 9d8e5af55..7358d4a59 100644
--- a/src/lib/buildgraph/jscommandexecutor.cpp
+++ b/src/lib/buildgraph/jscommandexecutor.cpp
@@ -40,9 +40,8 @@
 #include <tools/codelocation.h>
 #include <tools/error.h>
 
-#include <QtConcurrentRun>
 #include <QDir>
-#include <QFutureWatcher>
+#include <QEventLoop>
 #include <QHash>
 #include <QMutex>
 #include <QMutexLocker>
@@ -52,33 +51,35 @@
 namespace qbs {
 namespace Internal {
 
-struct JavaScriptCommandFutureResult
+struct JavaScriptCommandResult
 {
     bool success;
     QString errorMessage;
     CodeLocation errorLocation;
 };
 
-class JavaScriptCommandFutureWatcher : public QFutureWatcher<JavaScriptCommandFutureResult>
+class JsCommandExecutorThreadObject : public QObject
 {
+    Q_OBJECT
 public:
-    JavaScriptCommandFutureWatcher(QObject *parent)
-        : QFutureWatcher<JavaScriptCommandFutureResult>(parent)
-    {}
-};
+    JsCommandExecutorThreadObject(const Logger &logger)
+        : m_logger(logger)
+    {
+    }
 
-class JSRunner
-{
-public:
-    typedef JavaScriptCommandFutureResult result_type;
+    const JavaScriptCommandResult &result() const
+    {
+        return m_result;
+    }
 
-    JSRunner(const JavaScriptCommand *jsCommand, const Logger &logger)
-        : m_jsCommand(jsCommand), m_logger(logger) {}
+signals:
+    void finished();
 
-    JavaScriptCommandFutureResult operator() (Transformer *transformer)
+public slots:
+    void start(const JavaScriptCommand *cmd, Transformer *transformer)
     {
-        result_type result;
-        result.success = true;
+        m_result.success = true;
+        m_result.errorMessage.clear();
         ScriptEngine * const scriptEngine = lookupEngine();
         QString trafoPtrStr = QString::number((qulonglong)transformer);
         if (scriptEngine->globalObject().property("_qbs_transformer_ptr").toString() != trafoPtrStr) {
@@ -95,23 +96,27 @@ public:
         }
 
         scriptEngine->pushContext();
-        for (QVariantMap::const_iterator it = m_jsCommand->properties().constBegin(); it != m_jsCommand->properties().constEnd(); ++it)
-            scriptEngine->currentContext()->activationObject().setProperty(it.key(), scriptEngine->toScriptValue(it.value()));
+        for (QVariantMap::const_iterator it = cmd->properties().constBegin();
+                it != cmd->properties().constEnd(); ++it) {
+            scriptEngine->currentContext()->activationObject().setProperty(it.key(),
+                    scriptEngine->toScriptValue(it.value()));
+        }
 
-        scriptEngine->evaluate(m_jsCommand->sourceCode());
+        scriptEngine->evaluate(cmd->sourceCode());
         if (scriptEngine->hasUncaughtException()) {
-            result.success = false;
-            result.errorMessage = scriptEngine->uncaughtException().toString();
-            const CodeLocation &origLocation = m_jsCommand->codeLocation();
-            result.errorLocation = CodeLocation(origLocation.fileName(),
+            m_result.success = false;
+            m_result.errorMessage = scriptEngine->uncaughtException().toString();
+            const CodeLocation &origLocation = cmd->codeLocation();
+            m_result.errorLocation = CodeLocation(origLocation.fileName(),
                     origLocation.line() + scriptEngine->uncaughtExceptionLineNumber(),
                     origLocation.column());
         }
         scriptEngine->popContext();
         scriptEngine->clearExceptions();
-        return result;
+        emit finished();
     }
 
+private:
     ScriptEngine *lookupEngine()
     {
         QThread * const currentThread = QThread::currentThread();
@@ -126,47 +131,63 @@ public:
         return scriptEngine;
     }
 
-private:
     static QHash<QThread *, ScriptEngine *> m_enginesPerThread;
     static QMutex m_cacheMutex;
-    const JavaScriptCommand *m_jsCommand;
     Logger m_logger;
+    JavaScriptCommandResult m_result;
 };
 
-QHash<QThread *, ScriptEngine *> JSRunner::m_enginesPerThread;
-QMutex JSRunner::m_cacheMutex;
+QHash<QThread *, ScriptEngine *> JsCommandExecutorThreadObject::m_enginesPerThread;
+QMutex JsCommandExecutorThreadObject::m_cacheMutex;
 
 
 JsCommandExecutor::JsCommandExecutor(const Logger &logger, QObject *parent)
     : AbstractCommandExecutor(logger, parent)
-    , m_jsFutureWatcher(0)
+    , m_thread(new QThread(this))
+    , m_objectInThread(new JsCommandExecutorThreadObject(logger))
+    , m_running(false)
 {
+    m_objectInThread->moveToThread(m_thread);
+    connect(m_objectInThread, SIGNAL(finished()), this, SLOT(onJavaScriptCommandFinished()));
+    connect(this, SIGNAL(startRequested(const JavaScriptCommand *, Transformer *)),
+            m_objectInThread, SLOT(start(const JavaScriptCommand *, Transformer *)));
+}
+
+JsCommandExecutor::~JsCommandExecutor()
+{
+    waitForFinished();
+    delete m_objectInThread;
+    m_thread->quit();
+    m_thread->wait();
 }
 
 void JsCommandExecutor::waitForFinished()
 {
-    if (m_jsFutureWatcher && m_jsFutureWatcher->isRunning())
-        m_jsFutureWatcher->waitForFinished();
+    if (!m_running)
+        return;
+    QEventLoop loop;
+    loop.connect(m_objectInThread, SIGNAL(finished()), SLOT(quit()));
+    loop.exec();
 }
 
 void JsCommandExecutor::doStart()
 {
+    QBS_ASSERT(!m_running, return);
+    m_thread->start();
+
     if (dryRun()) {
         QTimer::singleShot(0, this, SIGNAL(finished())); // Don't call back on the caller.
         return;
     }
-    QFuture<JSRunner::result_type> future
-            = QtConcurrent::run(JSRunner(jsCommand(), logger()), transformer());
-    if (!m_jsFutureWatcher) {
-        m_jsFutureWatcher = new JavaScriptCommandFutureWatcher(this);
-        connect(m_jsFutureWatcher, SIGNAL(finished()), SLOT(onJavaScriptCommandFinished()));
-    }
-    m_jsFutureWatcher->setFuture(future);
+
+    m_running = true;
+    emit startRequested(jsCommand(), transformer());
 }
 
 void JsCommandExecutor::onJavaScriptCommandFinished()
 {
-    JavaScriptCommandFutureResult result = m_jsFutureWatcher->future().result();
+    m_running = false;
+    const JavaScriptCommandResult &result = m_objectInThread->result();
     if (!result.success) {
         logger().qbsDebug() << "JS context:\n" << jsCommand()->properties();
         logger().qbsDebug() << "JS code:\n" << jsCommand()->sourceCode();
@@ -184,3 +205,5 @@ const JavaScriptCommand *JsCommandExecutor::jsCommand() const
 
 } // namespace Internal
 } // namespace qbs
+
+#include <jscommandexecutor.moc>
diff --git a/src/lib/buildgraph/jscommandexecutor.h b/src/lib/buildgraph/jscommandexecutor.h
index f1ebefa6b..59cdf23e6 100644
--- a/src/lib/buildgraph/jscommandexecutor.h
+++ b/src/lib/buildgraph/jscommandexecutor.h
@@ -39,13 +39,17 @@ class CodeLocation;
 
 namespace Internal {
 class JavaScriptCommand;
-class JavaScriptCommandFutureWatcher;
+class JsCommandExecutorThreadObject;
 
 class JsCommandExecutor : public AbstractCommandExecutor
 {
     Q_OBJECT
 public:
     explicit JsCommandExecutor(const Logger &logger, QObject *parent = 0);
+    ~JsCommandExecutor();
+
+signals:
+    void startRequested(const JavaScriptCommand *cmd, Transformer *transformer);
 
 private slots:
     void onJavaScriptCommandFinished();
@@ -56,7 +60,9 @@ private:
 
     const JavaScriptCommand *jsCommand() const;
 
-    JavaScriptCommandFutureWatcher *m_jsFutureWatcher;
+    QThread *m_thread;
+    JsCommandExecutorThreadObject *m_objectInThread;
+    bool m_running;
 };
 
 } // namespace Internal