Make sure we destroy all QV4::QObjectWrapper objects

Delay freeing QObjectWrapper Value to MemoryManager::sweep() to make sure
we can destroy all QObjectWrapper objects.
We also keep track of QObjectWrapper in QV4::Heap::ModelObject to make
sure we destory them in QV4::MemoryManager::sweep()

Change-Id: I3a8a3b07faab1f88c2eb746f68aa8d9584b40026
Reviewed-by: Simon Hausmann <simon.hausmann@theqtcompany.com>

5 files changed, 44 insertions, 7 deletions

diff --git a/src/qml/jsruntime/qv4persistent.cpp b/src/qml/jsruntime/qv4persistent.cpp
index 4a0f84b685..1b0d6383e0 100644
--- a/src/qml/jsruntime/qv4persistent.cpp
+++ b/src/qml/jsruntime/qv4persistent.cpp
@@ -34,6 +34,7 @@
 #include "qv4persistent_p.h"
 #include <private/qv4mm_p.h>
 #include "qv4object_p.h"
+#include "qv4qobjectwrapper_p.h"
 #include "PageAllocation.h"
 
 using namespace QV4;
@@ -381,7 +382,7 @@ WeakValue &WeakValue::operator=(const WeakValue &other)
 
 WeakValue::~WeakValue()
 {
-    PersistentValueStorage::free(val);
+    free();
 }
 
 void WeakValue::set(ExecutionEngine *engine, const Value &value)
@@ -412,3 +413,21 @@ void WeakValue::markOnce(ExecutionEngine *e)
     val->mark(e);
 }
 
+void WeakValue::free()
+{
+    if (!val)
+        return;
+
+    ExecutionEngine *e = engine();
+    if (e && val->as<QObjectWrapper>()) {
+        // Some QV4::QObjectWrapper Value will be freed in WeakValue::~WeakValue() before MemoryManager::sweep() is being called,
+        // in this case we will never have a chance to call detroyObject() on those QV4::QObjectWrapper objects.
+        // Here we don't free these Value immediately, instead we keep track of them to free them later in MemoryManager::sweep()
+        e->memoryManager->m_pendingFreedObjectWrapperValue.push_back(val);
+    } else {
+        PersistentValueStorage::free(val);
+    }
+
+    val = 0;
+}
+
diff --git a/src/qml/jsruntime/qv4persistent_p.h b/src/qml/jsruntime/qv4persistent_p.h
index 80b4ecdea8..6345c9cc3d 100644
--- a/src/qml/jsruntime/qv4persistent_p.h
+++ b/src/qml/jsruntime/qv4persistent_p.h
@@ -178,15 +178,15 @@ public:
 
     bool isUndefined() const { return !val || val->isUndefined(); }
     bool isNullOrUndefined() const { return !val || val->isNullOrUndefined(); }
-    void clear() {
-        PersistentValueStorage::free(val);
-        val = 0;
-    }
+    void clear() { free(); }
 
     void markOnce(ExecutionEngine *e);
 
 private:
     Value *val;
+
+private:
+    void free();
 };
 
 } // namespace QV4
diff --git a/src/qml/memory/qv4mm.cpp b/src/qml/memory/qv4mm.cpp
index 5181bf912b..885784d7d3 100644
--- a/src/qml/memory/qv4mm.cpp
+++ b/src/qml/memory/qv4mm.cpp
@@ -41,8 +41,6 @@
 #include "StdLibExtras.h"
 
 #include <QTime>
-#include <QVector>
-#include <QVector>
 #include <QMap>
 
 #include <iostream>
@@ -437,6 +435,21 @@ void MemoryManager::sweep(bool lastSweep)
         (*it) = Primitive::undefinedValue();
     }
 
+    // Now it is time to free QV4::QObjectWrapper Value, we must check the Value's tag to make sure its object has been destroyed
+    const int pendingCount = m_pendingFreedObjectWrapperValue.count();
+    if (pendingCount) {
+        QVector<Value *> remainingWeakQObjectWrappers;
+        remainingWeakQObjectWrappers.reserve(pendingCount);
+        for (int i = 0; i < pendingCount; ++i) {
+            Value *v = m_pendingFreedObjectWrapperValue.at(i);
+            if (v->tag() == Value::Undefined_Type)
+                PersistentValueStorage::free(v);
+            else
+                remainingWeakQObjectWrappers.append(v);
+        }
+        m_pendingFreedObjectWrapperValue = remainingWeakQObjectWrappers;
+    }
+
     if (MultiplyWrappedQObjectMap *multiplyWrappedQObjects = engine->m_multiplyWrappedQObjects) {
         for (MultiplyWrappedQObjectMap::Iterator it = multiplyWrappedQObjects->begin(); it != multiplyWrappedQObjects->end();) {
             if (!it.value().isNullOrUndefined())
diff --git a/src/qml/memory/qv4mm_p.h b/src/qml/memory/qv4mm_p.h
index 3543da0907..e19cf301ea 100644
--- a/src/qml/memory/qv4mm_p.h
+++ b/src/qml/memory/qv4mm_p.h
@@ -49,6 +49,7 @@
 #include <private/qv4value_p.h>
 #include <private/qv4scopedvalue_p.h>
 #include <private/qv4object_p.h>
+#include <QVector>
 
 //#define DETAILED_MM_STATS
 
@@ -327,6 +328,7 @@ public:
     QScopedPointer<Data> m_d;
     PersistentValueStorage *m_persistentValues;
     PersistentValueStorage *m_weakValues;
+    QVector<Value *> m_pendingFreedObjectWrapperValue;
 };
 
 }
diff --git a/src/qml/types/qqmllistmodel.cpp b/src/qml/types/qqmllistmodel.cpp
index a4c0f7043f..4b0aa47c63 100644
--- a/src/qml/types/qqmllistmodel.cpp
+++ b/src/qml/types/qqmllistmodel.cpp
@@ -2277,6 +2277,9 @@ QQmlV4Handle QQmlListModel::get(int index) const
         } else {
             QObject *object = m_listModel->getOrCreateModelObject(const_cast<QQmlListModel *>(this), index);
             result = scope.engine->memoryManager->allocObject<QV4::ModelObject>(object, const_cast<QQmlListModel *>(this), index);
+            // Keep track of the QObjectWrapper in persistent value storage
+            QV4::Value *val = scope.engine->memoryManager->m_weakValues->allocate();
+            *val = result;
         }
     }