Guard QScriptEngine's creation/destruction with a mutex

When building multiple configurations in one go we create one
QScriptEngine per configuration, each in its own thread, but nearly at
the same time.

It turns out that QScriptEngine's creation/destruction is not
thread-safe, which leads to all kinds of interesting effects in the
multiple configurations case.

We fix this by guarding creation/destruction of QScriptEngine with a
mutex.

Task-number: QBS-1308
Change-Id: Ie01733d5943f1fb89e1d25344c4ffa998095e394
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>
Reviewed-by: Joerg Bornemann <joerg.bornemann@qt.io>

4 files changed, 17 insertions, 3 deletions

diff --git a/src/lib/corelib/buildgraph/jscommandexecutor.cpp b/src/lib/corelib/buildgraph/jscommandexecutor.cpp
index 3b8e9ab42..1a7224afc 100644
--- a/src/lib/corelib/buildgraph/jscommandexecutor.cpp
+++ b/src/lib/corelib/buildgraph/jscommandexecutor.cpp
@@ -167,7 +167,7 @@ private:
     ScriptEngine *provideScriptEngine()
     {
         if (!m_scriptEngine)
-            m_scriptEngine = new ScriptEngine(m_logger, EvalContext::JsCommand, this);
+            m_scriptEngine = ScriptEngine::create(m_logger, EvalContext::JsCommand, this);
         return m_scriptEngine;
     }
 
diff --git a/src/lib/corelib/buildgraph/rulesevaluationcontext.cpp b/src/lib/corelib/buildgraph/rulesevaluationcontext.cpp
index 0b6774adb..c81e40c4f 100644
--- a/src/lib/corelib/buildgraph/rulesevaluationcontext.cpp
+++ b/src/lib/corelib/buildgraph/rulesevaluationcontext.cpp
@@ -55,7 +55,7 @@ namespace Internal {
 
 RulesEvaluationContext::RulesEvaluationContext(const Logger &logger)
     : m_logger(logger),
-      m_engine(new ScriptEngine(m_logger, EvalContext::RuleExecution)),
+      m_engine(ScriptEngine::create(m_logger, EvalContext::RuleExecution)),
       m_observer(nullptr),
       m_initScopeCalls(0)
 {
diff --git a/src/lib/corelib/language/scriptengine.cpp b/src/lib/corelib/language/scriptengine.cpp
index b1c6d1410..8f31b879f 100644
--- a/src/lib/corelib/language/scriptengine.cpp
+++ b/src/lib/corelib/language/scriptengine.cpp
@@ -97,6 +97,8 @@ uint qHash(const ScriptEngine::PropertyCacheKey &k, uint seed = 0)
                        combineHash(qHash(k.m_propertyName), qHash(k.m_propertyMap), seed), seed);
 }
 
+std::mutex ScriptEngine::m_creationDestructionMutex;
+
 ScriptEngine::ScriptEngine(Logger &logger, EvalContext evalContext, QObject *parent)
     : QScriptEngine(parent), m_scriptImporter(new ScriptImporter(this)),
       m_modulePropertyScriptClass(nullptr),
@@ -116,8 +118,17 @@ ScriptEngine::ScriptEngine(Logger &logger, EvalContext evalContext, QObject *par
     extendJavaScriptBuiltins();
 }
 
+ScriptEngine *ScriptEngine::create(Logger &logger, EvalContext evalContext, QObject *parent)
+{
+    std::lock_guard<std::mutex> lock(m_creationDestructionMutex);
+    return new ScriptEngine(logger, evalContext, parent);
+}
+
 ScriptEngine::~ScriptEngine()
 {
+    m_creationDestructionMutex.lock();
+    connect(this, &QObject::destroyed, std::bind(&std::mutex::unlock, &m_creationDestructionMutex));
+
     releaseResourcesOfScriptObjects();
     delete (m_scriptImporter);
     if (m_elapsedTimeImporting != -1) {
diff --git a/src/lib/corelib/language/scriptengine.h b/src/lib/corelib/language/scriptengine.h
index 05f4f519f..c01adbcbe 100644
--- a/src/lib/corelib/language/scriptengine.h
+++ b/src/lib/corelib/language/scriptengine.h
@@ -56,6 +56,7 @@
 #include <QtScript/qscriptengine.h>
 
 #include <memory>
+#include <mutex>
 #include <stack>
 #include <tuple>
 #include <unordered_map>
@@ -96,8 +97,9 @@ enum class ObserveMode { Enabled, Disabled };
 class QBS_AUTOTEST_EXPORT ScriptEngine : public QScriptEngine
 {
     Q_OBJECT
-public:
     ScriptEngine(Logger &logger, EvalContext evalContext, QObject *parent = nullptr);
+public:
+    static ScriptEngine *create(Logger &logger, EvalContext evalContext, QObject *parent = nullptr);
     ~ScriptEngine();
 
     Logger &logger() const { return m_logger; }
@@ -244,6 +246,7 @@ private:
     friend bool operator==(const PropertyCacheKey &lhs, const PropertyCacheKey &rhs);
     friend uint qHash(const ScriptEngine::PropertyCacheKey &k, uint seed);
 
+    static std::mutex m_creationDestructionMutex;
     ScriptImporter *m_scriptImporter;
     QScriptClass *m_modulePropertyScriptClass;
     QHash<JsImport, QScriptValue> m_jsImportCache;