Fix crashes caused by handle management in worker threads

Previously, the QV8Engine destructor and the QV8Engine's QV8GCCallback
Referencer destructor would crash if run after the v8 isolate had been
exited and disposed.  This commit Q_ASSERTs if the worker thread
attempts to do so, and adds a cleanup function which worker threads
should call just prior to exiting the isolate.

Task-number: QTBUG-21866
Change-Id: I379b02e24ad9378e4bfc270fb9208715b6f7b60a
Reviewed-on: http://codereview.qt-project.org/6202
Reviewed-by: Toby Tomkins <toby.tomkins@nokia.com>
Sanity-Review: Qt Sanity Bot <qt_sanity_bot@ovi.com>
Reviewed-by: Aaron Kennedy <aaron.kennedy@nokia.com>

diff --git a/src/declarative/qml/qdeclarativeworkerscript.cpp b/src/declarative/qml/qdeclarativeworkerscript.cpp
index e6af0ba..c6425d0 100644 (file)
--- a/src/declarative/qml/qdeclarativeworkerscript.cpp
+++ b/src/declarative/qml/qdeclarativeworkerscript.cpp
@@ -59,6 +59,7 @@
 
 #include <private/qv8engine_p.h>
 #include <private/qv8worker_p.h>
+#include <private/qv8gccallback_p.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -542,7 +543,7 @@ void QDeclarativeWorkerScriptEngine::run()
     d->workers.clear();
 
     delete d->workerEngine; d->workerEngine = 0;
-
+    QV8GCCallback::releaseWorkerThreadGcPrologueCallbackData();
     isolate->Exit();
     isolate->Dispose();
 }

diff --git a/src/declarative/qml/v8/qv8engine.cpp b/src/declarative/qml/v8/qv8engine.cpp
index 7012ae1..e135ec9 100644 (file)
--- a/src/declarative/qml/v8/qv8engine.cpp
+++ b/src/declarative/qml/v8/qv8engine.cpp
@@ -146,6 +146,7 @@ QV8Engine::QV8Engine(QJSEngine* qq, QJSEngine::ContextOwnership ownership)
 
 QV8Engine::~QV8Engine()
 {
+    Q_ASSERT_X(v8::Isolate::GetCurrent(), "QV8Engine::~QV8Engine()", "called after v8::Isolate has exited");
     for (int ii = 0; ii < m_extensionData.count(); ++ii) 
         delete m_extensionData[ii];
     m_extensionData.clear();
@@ -1383,6 +1384,26 @@ void QV8GCCallback::registerGcPrologueCallback()
     }
 }
 
+void QV8GCCallback::ThreadData::releaseStrongReferencer()
+{
+    // NOTE: must be called with a valid current isolate
+    if (!referencer.strongReferencer.IsEmpty()) {
+        qPersistentDispose(referencer.strongReferencer);
+    }
+}
+
+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->releaseStrongReferencer();
+    }
+}
+
 QV8GCCallback::Node::Node(PrologueCallback callback)
     : prologueCallback(callback)
 {
@@ -1395,7 +1416,12 @@ QV8GCCallback::Node::~Node()
 
 QV8GCCallback::Referencer::~Referencer()
 {
-    qPersistentDispose(strongReferencer);
+    if (!strongReferencer.IsEmpty()) {
+        Q_ASSERT_X(v8::Isolate::GetCurrent(), "QV8GCCallback::Referencer::~Referencer()", "called after v8::Isolate has exited");
+        // automatically release the strongReferencer if it hasn't
+        // been explicitly released already.
+        qPersistentDispose(strongReferencer);
+    }
 }
 
 QV8GCCallback::Referencer::Referencer()

diff --git a/src/declarative/qml/v8/qv8gccallback_p.h b/src/declarative/qml/v8/qv8gccallback_p.h
index f5f7d8b..91574cb 100644 (file)
--- a/src/declarative/qml/v8/qv8gccallback_p.h
+++ b/src/declarative/qml/v8/qv8gccallback_p.h
@@ -67,6 +67,7 @@ private:
 public:
     static void garbageCollectorPrologueCallback(v8::GCType, v8::GCCallbackFlags);
     static void registerGcPrologueCallback();
+    static void releaseWorkerThreadGcPrologueCallbackData();
 
     class Referencer {
     public:
@@ -100,6 +101,7 @@ private:
         Referencer referencer;
         bool gcPrologueCallbackRegistered;
         QIntrusiveList<Node, &Node::node> gcCallbackNodes;
+        void releaseStrongReferencer();
     };
 
     static void initializeThreadData();