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

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>

diff --git a/src/qml/jsruntime/qv4mm.cpp b/src/qml/jsruntime/qv4mm.cpp
index 07408a343c7595b8aac1b6f56d3959bc152b640b..d5576b400a4af76a861de9abdf2e829f198a6bc6 100644 (file)
--- 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 50efff001d0fcd108c9ebcbff0883c8b90a85bdc..353f4023ca900f5654b014c0b7c827d5c3ea91e8 100644 (file)
--- 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 1b41ca65c16d93bfa486d88118e9d1abe0a18e03..5d2378018cc61610ad7ad44336df8877def73bd4 100644 (file)
--- 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 c7654be5459ff384a7f57e58628607e0edc5c0fe..04c42b638ddd9a959bead6dbda6ec6d226b7ec56 100644 (file)
--- 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,