Fix that QJSEngine cannot be used from threads other than the gui thread

Implicitly allocate & enter an isolate per thread if needed, store it in TLS and
exit it upon thread destruction. As the code that represents QObject
dependencies in the GC through implicit dependencies uses its own context,
its per-thread data is folded into the v8engine TLS to ensure that it is
destructed before the isolate is exited.

Task-number: QTBUG-23099

Change-Id: I86538b54939b2fe64db843052eac04c7fd31813e
Reviewed-by: Aaron Kennedy <aaron.kennedy@nokia.com>

src/declarative/qml/qdeclarativeworkerscript.cpp

@@ -526,9 +526,6 @@ void QDeclarativeWorkerScriptEngine::run()
 {
     d->m_lock.lock();
 
-    v8::Isolate *isolate = v8::Isolate::New(); 
-    isolate->Enter();
-
     d->workerEngine = new QDeclarativeWorkerScriptEnginePrivate::WorkerEngine(d);
     d->workerEngine->init();
 
@@ -542,9 +539,6 @@ void QDeclarativeWorkerScriptEngine::run()
     d->workers.clear();
 
     delete d->workerEngine; d->workerEngine = 0;
-    QV8GCCallback::releaseWorkerThreadGcPrologueCallbackData();
-    isolate->Exit();
-    isolate->Dispose();
 }
 
 

src/declarative/qml/v8/qv8engine.cpp

@@ -131,6 +131,8 @@ QV8Engine::QV8Engine(QJSEngine* qq, QJSEngine::ContextOwnership ownership)
         v8args.append(" --nobreakpoint_relocation");
     v8::V8::SetFlagsFromString(v8args.constData(), v8args.length());
 
+    ensurePerThreadIsolate();
+
     v8::HandleScope handle_scope;
     m_context = (ownership == QJSEngine::CreateNewContext) ? v8::Context::New() : v8::Persistent<v8::Context>::New(v8::Context::GetCurrent());
     qPersistentRegister(m_context);
@@ -767,6 +769,25 @@ QDateTime QV8Engine::qtDateTimeFromJsDate(double jsDate)
     return convertedUTC.toLocalTime();
 }
 
+static QThreadStorage<QV8Engine::ThreadData*> perThreadEngineData;
+
+bool QV8Engine::hasThreadData()
+{
+    return perThreadEngineData.hasLocalData();
+}
+
+QV8Engine::ThreadData *QV8Engine::threadData()
+{
+    Q_ASSERT(perThreadEngineData.hasLocalData());
+    return perThreadEngineData.localData();
+}
+
+void QV8Engine::ensurePerThreadIsolate()
+{
+    if (!perThreadEngineData.hasLocalData())
+        perThreadEngineData.setLocalData(new ThreadData);
+}
+
 void QV8Engine::initDeclarativeGlobalObject()
 {
     v8::HandleScope handels;
@@ -1430,37 +1451,17 @@ qint64 QV8Engine::stopTimer(const QString &timerName, bool *wasRunning)
     return m_time.elapsed() - startedAt;
 }
 
-QThreadStorage<QV8GCCallback::ThreadData *> QV8GCCallback::threadData;
-void QV8GCCallback::initializeThreadData()
-{
-    QV8GCCallback::ThreadData *newThreadData = new QV8GCCallback::ThreadData;
-    threadData.setLocalData(newThreadData);
-}
-
 void QV8GCCallback::registerGcPrologueCallback()
 {
-    if (!threadData.hasLocalData())
-        initializeThreadData();
-
-    QV8GCCallback::ThreadData *td = threadData.localData();
+    QV8Engine::ThreadData *td = QV8Engine::threadData();
     if (!td->gcPrologueCallbackRegistered) {
         td->gcPrologueCallbackRegistered = true;
+        if (!td->referencer)
+            td->referencer = new QV8GCCallback::Referencer;
         v8::V8::AddGCPrologueCallback(QV8GCCallback::garbageCollectorPrologueCallback, v8::kGCTypeMarkSweepCompact);
     }
 }
 
-void QV8GCCallback::releaseWorkerThreadGcPrologueCallbackData()
-{
-    // Note that only worker-thread implementations with their
-    // own QV8Engine should explicitly release the Referencer
-    // by calling this functions.
-    Q_ASSERT_X(v8::Isolate::GetCurrent(), "QV8GCCallback::releaseWorkerThreadGcPrologueCallbackData()", "called after v8::Isolate has exited");
-    if (threadData.hasLocalData()) {
-        QV8GCCallback::ThreadData *td = threadData.localData();
-        td->referencer.dispose();
-    }
-}
-
 QV8GCCallback::Node::Node(PrologueCallback callback)
     : prologueCallback(callback)
 {
@@ -1569,32 +1570,49 @@ v8::Persistent<v8::Object> *QV8GCCallback::Referencer::findOwnerAndStrength(QObj
  */
 void QV8GCCallback::garbageCollectorPrologueCallback(v8::GCType, v8::GCCallbackFlags)
 {
-    if (!threadData.hasLocalData())
+    if (!QV8Engine::hasThreadData())
         return;
 
-    QV8GCCallback::ThreadData *td = threadData.localData();
+    QV8Engine::ThreadData *td = QV8Engine::threadData();
+    Q_ASSERT(td->referencer);
     QV8GCCallback::Node *currNode = td->gcCallbackNodes.first();
 
     while (currNode) {
         // The client which adds itself to the list is responsible
         // for maintaining the correct implicit references in the
         // specified callback.
-        currNode->prologueCallback(&td->referencer, currNode);
+        currNode->prologueCallback(td->referencer, currNode);
         currNode = td->gcCallbackNodes.next(currNode);
     }
 }
 
 void QV8GCCallback::addGcCallbackNode(QV8GCCallback::Node *node)
 {
-    if (!threadData.hasLocalData())
-        initializeThreadData();
-
-    QV8GCCallback::ThreadData *td = threadData.localData();
+    QV8Engine::ThreadData *td = QV8Engine::threadData();
     td->gcCallbackNodes.insert(node);
 }
 
-QV8GCCallback::ThreadData::~ThreadData()
+QV8Engine::ThreadData::ThreadData()
+    : referencer(0)
+    , gcPrologueCallbackRegistered(false)
 {
+    if (!v8::Isolate::GetCurrent()) {
+        isolate = v8::Isolate::New();
+        isolate->Enter();
+    } else {
+        isolate = 0;
+    }
+}
+
+QV8Engine::ThreadData::~ThreadData()
+{
+    delete referencer;
+    referencer = 0;
+    if (isolate) {
+        isolate->Exit();
+        isolate->Dispose();
+        isolate = 0;
+    }
 }
 
 QT_END_NAMESPACE

src/declarative/qml/v8/qv8engine_p.h

@@ -230,7 +230,6 @@ class Q_AUTOTEST_EXPORT QV8GCCallback
 public:
     static void garbageCollectorPrologueCallback(v8::GCType, v8::GCCallbackFlags);
     static void registerGcPrologueCallback();
-    static void releaseWorkerThreadGcPrologueCallbackData();
 
     class Q_AUTOTEST_EXPORT Referencer {
     public:
@@ -243,7 +242,7 @@ class Q_AUTOTEST_EXPORT QV8GCCallback
         static v8::Persistent<v8::Object> *findOwnerAndStrength(QObject *qobjectOwner, bool *shouldBeStrong);
         v8::Persistent<v8::Object> strongReferencer;
         v8::Persistent<v8::Context> context;
-        friend class QV8GCCallback::ThreadData;
+        friend class QV8GCCallback;
     };
 
     class Q_AUTOTEST_EXPORT Node {
@@ -257,19 +256,6 @@ class Q_AUTOTEST_EXPORT QV8GCCallback
     };
 
     static void addGcCallbackNode(Node *node);
-
-private:
-    class ThreadData {
-    public:
-        ThreadData() : gcPrologueCallbackRegistered(false) { }
-        ~ThreadData();
-        Referencer referencer;
-        bool gcPrologueCallbackRegistered;
-        QIntrusiveList<Node, &Node::node> gcCallbackNodes;
-    };
-
-    static void initializeThreadData();
-    static QThreadStorage<ThreadData *> threadData;
 };
 
 class Q_DECLARATIVE_EXPORT QV8Engine
@@ -465,6 +451,19 @@ class Q_DECLARATIVE_EXPORT QV8Engine
 
     static QDateTime qtDateTimeFromJsDate(double jsDate);
 
+    struct ThreadData {
+        ThreadData();
+        ~ThreadData();
+        v8::Isolate* isolate;
+        QV8GCCallback::Referencer* referencer;
+        bool gcPrologueCallbackRegistered;
+        QIntrusiveList<QV8GCCallback::Node, &QV8GCCallback::Node::node> gcCallbackNodes;
+    };
+
+    static bool hasThreadData();
+    static ThreadData* threadData();
+    static void ensurePerThreadIsolate();
+
 protected:
     QJSEngine* q;
     QDeclarativeEngine *m_engine;

tests/auto/declarative/qjsengine/tst_qjsengine.cpp

@@ -315,6 +315,7 @@ private slots:
     void dateConversionJSQt();
     void dateConversionQtJS();
     void functionPrototypeExtensions();
+    void threadedEngine();
 };
 
 tst_QJSEngine::tst_QJSEngine()
@@ -6488,6 +6489,35 @@ void tst_QJSEngine::functionPrototypeExtensions()
     QCOMPARE(props.property("length").toInt32(), 0);
 }
 
+class ThreadedTestEngine : public QThread {
+    Q_OBJECT;
+
+public:
+    int result;
+
+    ThreadedTestEngine()
+        : result(0) {}
+
+    void run() {
+        QJSEngine firstEngine;
+        QJSEngine secondEngine;
+        QJSValue value = firstEngine.evaluate("1");
+        result = secondEngine.evaluate("1 + " + QString::number(value.toInteger())).toInteger();
+    }
+};
+
+void tst_QJSEngine::threadedEngine()
+{
+    ThreadedTestEngine thread1;
+    ThreadedTestEngine thread2;
+    thread1.start();
+    thread2.start();
+    thread1.wait();
+    thread2.wait();
+    QCOMPARE(thread1.result, 2);
+    QCOMPARE(thread2.result, 2);
+}
+
 QTEST_MAIN(tst_QJSEngine)
 
 #include "tst_qjsengine.moc"