Fix memory corruption when multiple QML engines have JavaScript wrappers for the same QObject

It's possible that the same QObject is exposed to multiple JavaScript
environments, for which we have this "extra" hack in the form of a QMap.  The
common case is that QQmlData has a QV4::WeakValue that points to the JS wrapper
for the object. However in the rare case of multiple exposure, a map in the
other engines stores those references. That map was erroneously storing
pointers to temporary values on the JS stack instead of heap pointers.

Change-Id: I8587f9921a9b4f9efd288326d00cebc25ad0bc12
Task-number: QTBUG-45051
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

4 files changed, 25 insertions, 21 deletions

diff --git a/src/qml/jsruntime/qv4mm.cpp b/src/qml/jsruntime/qv4mm.cpp
index 07408a343c..d5576b400a 100644
--- a/src/qml/jsruntime/qv4mm.cpp
+++ b/src/qml/jsruntime/qv4mm.cpp
@@ -390,7 +390,7 @@ void MemoryManager::sweep(bool lastSweep)
 
     if (MultiplyWrappedQObjectMap *multiplyWrappedQObjects = m_d->engine->m_multiplyWrappedQObjects) {
         for (MultiplyWrappedQObjectMap::Iterator it = multiplyWrappedQObjects->begin(); it != multiplyWrappedQObjects->end();) {
-            if (!it.value()->markBit())
+            if (!it.value().isNullOrUndefined())
                 it = multiplyWrappedQObjects->erase(it);
             else
                 ++it;
diff --git a/src/qml/jsruntime/qv4qobjectwrapper.cpp b/src/qml/jsruntime/qv4qobjectwrapper.cpp
index 50efff001d..353f4023ca 100644
--- a/src/qml/jsruntime/qv4qobjectwrapper.cpp
+++ b/src/qml/jsruntime/qv4qobjectwrapper.cpp
@@ -589,7 +589,7 @@ ReturnedValue QObjectWrapper::wrap(ExecutionEngine *engine, QObject *object)
     } else if (ddata->jsWrapper.isUndefined() &&
                (ddata->jsEngineId == engine->m_engineId || // We own the QObject
                 ddata->jsEngineId == 0 ||    // No one owns the QObject
-                !ddata->hasTaintedV8Object)) { // Someone else has used the QObject, but it isn't tainted
+                !ddata->hasTaintedV4Object)) { // Someone else has used the QObject, but it isn't tainted
 
         QV4::ScopedValue rv(scope, create(engine, object));
         ddata->jsWrapper.set(scope.engine, rv);
@@ -600,11 +600,11 @@ ReturnedValue QObjectWrapper::wrap(ExecutionEngine *engine, QObject *object)
         // If this object is tainted, we have to check to see if it is in our
         // tainted object list
         ScopedObject alternateWrapper(scope, (Object *)0);
-        if (engine->m_multiplyWrappedQObjects && ddata->hasTaintedV8Object)
+        if (engine->m_multiplyWrappedQObjects && ddata->hasTaintedV4Object)
             alternateWrapper = engine->m_multiplyWrappedQObjects->value(object);
 
         // If our tainted handle doesn't exist or has been collected, and there isn't
-        // a handle in the ddata, we can assume ownership of the ddata->v8object
+        // a handle in the ddata, we can assume ownership of the ddata->jsWrapper
         if (ddata->jsWrapper.isUndefined() && !alternateWrapper) {
             QV4::ScopedValue result(scope, create(engine, object));
             ddata->jsWrapper.set(scope.engine, result);
@@ -616,8 +616,8 @@ ReturnedValue QObjectWrapper::wrap(ExecutionEngine *engine, QObject *object)
             alternateWrapper = create(engine, object);
             if (!engine->m_multiplyWrappedQObjects)
                 engine->m_multiplyWrappedQObjects = new MultiplyWrappedQObjectMap;
-            engine->m_multiplyWrappedQObjects->insert(object, alternateWrapper);
-            ddata->hasTaintedV8Object = true;
+            engine->m_multiplyWrappedQObjects->insert(object, alternateWrapper->d());
+            ddata->hasTaintedV4Object = true;
         }
 
         return alternateWrapper.asReturnedValue();
@@ -1908,16 +1908,20 @@ Heap::QmlSignalHandler::QmlSignalHandler(QV4::ExecutionEngine *engine, QObject *
 
 DEFINE_OBJECT_VTABLE(QmlSignalHandler);
 
-void MultiplyWrappedQObjectMap::insert(QObject *key, Object *value)
+void MultiplyWrappedQObjectMap::insert(QObject *key, Heap::Object *value)
 {
-    QHash<QObject*, Object*>::insert(key, value);
+    QV4::WeakValue v;
+    v.set(value->internalClass->engine, value);
+    QHash<QObject*, QV4::WeakValue>::insert(key, v);
     connect(key, SIGNAL(destroyed(QObject*)), this, SLOT(removeDestroyedObject(QObject*)));
 }
 
+
+
 MultiplyWrappedQObjectMap::Iterator MultiplyWrappedQObjectMap::erase(MultiplyWrappedQObjectMap::Iterator it)
 {
     disconnect(it.key(), SIGNAL(destroyed(QObject*)), this, SLOT(removeDestroyedObject(QObject*)));
-    return QHash<QObject*, Object*>::erase(it);
+    return QHash<QObject*, QV4::WeakValue>::erase(it);
 }
 
 void MultiplyWrappedQObjectMap::remove(QObject *key)
@@ -1930,7 +1934,7 @@ void MultiplyWrappedQObjectMap::remove(QObject *key)
 
 void MultiplyWrappedQObjectMap::removeDestroyedObject(QObject *object)
 {
-    QHash<QObject*, Object*>::remove(object);
+    QHash<QObject*, QV4::WeakValue>::remove(object);
 }
 
 QT_END_NAMESPACE
diff --git a/src/qml/jsruntime/qv4qobjectwrapper_p.h b/src/qml/jsruntime/qv4qobjectwrapper_p.h
index 1b41ca65c1..5d2378018c 100644
--- a/src/qml/jsruntime/qv4qobjectwrapper_p.h
+++ b/src/qml/jsruntime/qv4qobjectwrapper_p.h
@@ -173,20 +173,20 @@ struct QmlSignalHandler : public QV4::Object
 };
 
 class MultiplyWrappedQObjectMap : public QObject,
-                                  private QHash<QObject*, Object*>
+                                  private QHash<QObject*, QV4::WeakValue>
 {
     Q_OBJECT
 public:
-    typedef QHash<QObject*, Object*>::ConstIterator ConstIterator;
-    typedef QHash<QObject*, Object*>::Iterator Iterator;
+    typedef QHash<QObject*, QV4::WeakValue>::ConstIterator ConstIterator;
+    typedef QHash<QObject*, QV4::WeakValue>::Iterator Iterator;
 
-    ConstIterator begin() const { return QHash<QObject*, Object*>::constBegin(); }
-    Iterator begin() { return QHash<QObject*, Object*>::begin(); }
-    ConstIterator end() const { return QHash<QObject*, Object*>::constEnd(); }
-    Iterator end() { return QHash<QObject*, Object*>::end(); }
+    ConstIterator begin() const { return QHash<QObject*, QV4::WeakValue>::constBegin(); }
+    Iterator begin() { return QHash<QObject*, QV4::WeakValue>::begin(); }
+    ConstIterator end() const { return QHash<QObject*, QV4::WeakValue>::constEnd(); }
+    Iterator end() { return QHash<QObject*, QV4::WeakValue>::end(); }
 
-    void insert(QObject *key, Object *value);
-    Object *value(QObject *key) const { return QHash<QObject*, Object*>::value(key, 0); }
+    void insert(QObject *key, Heap::Object *value);
+    ReturnedValue value(QObject *key) const { return QHash<QObject*, QV4::WeakValue>::value(key).value(); }
     Iterator erase(Iterator it);
     void remove(QObject *key);
 
diff --git a/src/qml/qml/qqmldata_p.h b/src/qml/qml/qqmldata_p.h
index c7654be545..04c42b638d 100644
--- a/src/qml/qml/qqmldata_p.h
+++ b/src/qml/qml/qqmldata_p.h
@@ -74,7 +74,7 @@ class Q_QML_PRIVATE_EXPORT QQmlData : public QAbstractDeclarativeData
 public:
     QQmlData()
         : ownedByQml1(false), ownMemory(true), ownContext(false), indestructible(true), explicitIndestructibleSet(false),
-          hasTaintedV8Object(false), isQueuedForDeletion(false), rootObjectInCreation(false),
+          hasTaintedV4Object(false), isQueuedForDeletion(false), rootObjectInCreation(false),
           hasVMEMetaObject(false), parentFrozen(false), bindingBitsSize(0), bindingBits(0), notifyList(0), context(0), outerContext(0),
           bindings(0), signalHandlers(0), nextContextObject(0), prevContextObject(0),
           lineNumber(0), columnNumber(0), jsEngineId(0), compiledData(0), deferredData(0),
@@ -112,7 +112,7 @@ public:
     quint32 ownContext:1;
     quint32 indestructible:1;
     quint32 explicitIndestructibleSet:1;
-    quint32 hasTaintedV8Object:1;
+    quint32 hasTaintedV4Object:1;
     quint32 isQueuedForDeletion:1;
     /*
      * rootObjectInCreation should be true only when creating top level CPP and QML objects,