destruct qobject wrappers before sweeping the GC heap

The wrappers emit a destroyed signal, and it's important
that the GC heap is in a well defined state when these signals
are emitted.

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

5 files changed, 40 insertions, 79 deletions

diff --git a/src/qml/jsruntime/qv4managed_p.h b/src/qml/jsruntime/qv4managed_p.h
index 9b7df9e68e..6f5564300f 100644
--- a/src/qml/jsruntime/qv4managed_p.h
+++ b/src/qml/jsruntime/qv4managed_p.h
@@ -78,14 +78,6 @@ inline void qYouForgotTheQ_MANAGED_Macro(T1, T2) {}
     (classname::func == QV4::Managed::func ? 0 : classname::func)
 
 
-struct GCDeletable
-{
-    GCDeletable() : next(0), lastCall(false) {}
-    virtual ~GCDeletable() {}
-    GCDeletable *next;
-    bool lastCall;
-};
-
 #define DEFINE_MANAGED_VTABLE_INT(classname, parentVTable) \
 {     \
     parentVTable, \
diff --git a/src/qml/jsruntime/qv4qobjectwrapper.cpp b/src/qml/jsruntime/qv4qobjectwrapper.cpp
index 5dc1cfef10..31abab2e2f 100644
--- a/src/qml/jsruntime/qv4qobjectwrapper.cpp
+++ b/src/qml/jsruntime/qv4qobjectwrapper.cpp
@@ -1037,48 +1037,31 @@ void QObjectWrapper::markObjects(Heap::Base *that, QV4::ExecutionEngine *e)
     QV4::Object::markObjects(that, e);
 }
 
-namespace {
-    struct QObjectDeleter : public QV4::GCDeletable
-    {
-        QObjectDeleter(QObject *o)
-            : m_objectToDelete(o)
-        {}
-        ~QObjectDeleter()
-        {
-            QQmlData *ddata = QQmlData::get(m_objectToDelete, false);
-            if (ddata && ddata->ownContext && ddata->context)
-                ddata->context->emitDestruction();
-            // This object is notionally destroyed now
-            ddata->isQueuedForDeletion = true;
-            if (lastCall)
-                delete m_objectToDelete;
-            else
-                m_objectToDelete->deleteLater();
-        }
-
-        QObject *m_objectToDelete;
-    };
-}
-
-void QObjectWrapper::destroy(Heap::Base *that)
+void QObjectWrapper::destroyObject(bool lastCall)
 {
-    Heap::QObjectWrapper *This = static_cast<Heap::QObjectWrapper*>(that);
-    QPointer<QObject> object = This->object;
-    ExecutionEngine *engine = This->internalClass->engine;
-    This->~Data();
-    This = 0;
-    if (!object)
-        return;
-
-    QQmlData *ddata = QQmlData::get(object, false);
-    if (!ddata)
-        return;
-
-    if (object->parent() || ddata->indestructible)
-        return;
+    Heap::QObjectWrapper *h = d();
+    if (!h->internalClass)
+        return; // destroyObject already got called
+
+    QPointer<QObject> object = h->object;
+    if (object) {
+        QQmlData *ddata = QQmlData::get(object, false);
+        if (ddata) {
+            if (!object->parent() && !ddata->indestructible) {
+                if (ddata && ddata->ownContext && ddata->context)
+                    ddata->context->emitDestruction();
+                // This object is notionally destroyed now
+                ddata->isQueuedForDeletion = true;
+                if (lastCall)
+                    delete object;
+                else
+                    object->deleteLater();
+            }
+        }
+    }
 
-    QObjectDeleter *deleter = new QObjectDeleter(object);
-    engine->memoryManager->registerDeletable(deleter);
+    h->internalClass = 0;
+    h->~Data();
 }
 
 
diff --git a/src/qml/jsruntime/qv4qobjectwrapper_p.h b/src/qml/jsruntime/qv4qobjectwrapper_p.h
index 8b86300e26..2d10006d97 100644
--- a/src/qml/jsruntime/qv4qobjectwrapper_p.h
+++ b/src/qml/jsruntime/qv4qobjectwrapper_p.h
@@ -118,6 +118,8 @@ struct Q_QML_EXPORT QObjectWrapper : public Object
     static void setProperty(ExecutionEngine *engine, QObject *object, int propertyIndex, const Value &value);
     void setProperty(ExecutionEngine *engine, int propertyIndex, const Value &value);
 
+    void destroyObject(bool lastCall);
+
 protected:
     static bool isEqualTo(Managed *that, Managed *o);
 
@@ -134,7 +136,6 @@ private:
     static PropertyAttributes query(const Managed *, String *name);
     static void advanceIterator(Managed *m, ObjectIterator *it, Value *name, uint *index, Property *p, PropertyAttributes *attributes);
     static void markObjects(Heap::Base *that, QV4::ExecutionEngine *e);
-    static void destroy(Heap::Base *that);
 
     static ReturnedValue method_connect(CallContext *ctx);
     static ReturnedValue method_disconnect(CallContext *ctx);
diff --git a/src/qml/memory/qv4mm.cpp b/src/qml/memory/qv4mm.cpp
index 7e75570af3..24be663ed7 100644
--- a/src/qml/memory/qv4mm.cpp
+++ b/src/qml/memory/qv4mm.cpp
@@ -109,8 +109,6 @@ struct MemoryManager::Data
     LargeItem *largeItems;
     std::size_t totalLargeItemsAllocated;
 
-    GCDeletable *deletable;
-
     // statistics:
 #ifdef DETAILED_MM_STATS
     QVector<unsigned> allocSizeCounters;
@@ -125,7 +123,6 @@ struct MemoryManager::Data
         , maxChunkSize(32*1024)
         , largeItems(0)
         , totalLargeItemsAllocated(0)
-        , deletable(0)
     {
         memset(nonFullChunks, 0, sizeof(nonFullChunks));
         memset(nChunks, 0, sizeof(nChunks));
@@ -351,8 +348,6 @@ void MemoryManager::mark()
         if ((*it).managed()->d()->vtable() != QObjectWrapper::staticVTable())
             continue;
         QObjectWrapper *qobjectWrapper = static_cast<QObjectWrapper*>((*it).managed());
-        if (!qobjectWrapper)
-            continue;
         QObject *qobject = qobjectWrapper->object();
         if (!qobject)
             continue;
@@ -379,13 +374,20 @@ void MemoryManager::mark()
 
 void MemoryManager::sweep(bool lastSweep)
 {
-    if (m_weakValues) {
-        for (PersistentValueStorage::Iterator it = m_weakValues->begin(); it != m_weakValues->end(); ++it) {
-            if (Managed *m = (*it).as<Managed>()) {
-                if (!m->markBit())
-                    (*it) = Primitive::undefinedValue();
-            }
+    for (PersistentValueStorage::Iterator it = m_weakValues->begin(); it != m_weakValues->end(); ++it) {
+        if (!(*it).isManaged())
+            continue;
+        Managed *m = (*it).as<Managed>();
+        if (m->markBit())
+            continue;
+        // we need to call detroyObject on qobjectwrappers now, so that they can emit the destroyed
+        // signal before we start sweeping the heap
+        if ((*it).managed()->d()->vtable() == QObjectWrapper::staticVTable()) {
+            QObjectWrapper *qobjectWrapper = static_cast<QObjectWrapper*>((*it).managed());
+            qobjectWrapper->destroyObject(lastSweep);
         }
+
+        (*it) = Primitive::undefinedValue();
     }
 
     if (MultiplyWrappedQObjectMap *multiplyWrappedQObjects = m_d->engine->m_multiplyWrappedQObjects) {
@@ -453,15 +455,6 @@ void MemoryManager::sweep(bool lastSweep)
         i = *last;
     }
 
-    GCDeletable *deletable = m_d->deletable;
-    m_d->deletable = 0;
-    while (deletable) {
-        GCDeletable *next = deletable->next;
-        deletable->lastCall = lastSweep;
-        delete deletable;
-        deletable = next;
-    }
-
     // some execution contexts are allocated on the stack, make sure we clear their markBit as well
     if (!lastSweep) {
         Heap::ExecutionContext *ctx = engine()->current;
@@ -556,10 +549,10 @@ size_t MemoryManager::getLargeItemsMem() const
 MemoryManager::~MemoryManager()
 {
     delete m_persistentValues;
-    delete m_weakValues;
-    m_weakValues = 0;
 
     sweep(/*lastSweep*/true);
+
+    delete m_weakValues;
 #ifdef V4_USE_VALGRIND
     VALGRIND_DESTROY_MEMPOOL(this);
 #endif
@@ -584,12 +577,6 @@ void MemoryManager::dumpStats() const
 #endif // DETAILED_MM_STATS
 }
 
-void MemoryManager::registerDeletable(GCDeletable *d)
-{
-    d->next = m_d->deletable;
-    m_d->deletable = d;
-}
-
 #ifdef DETAILED_MM_STATS
 void MemoryManager::willAllocate(std::size_t size)
 {
diff --git a/src/qml/memory/qv4mm_p.h b/src/qml/memory/qv4mm_p.h
index a7b4e6ef4e..6d6ce1bad7 100644
--- a/src/qml/memory/qv4mm_p.h
+++ b/src/qml/memory/qv4mm_p.h
@@ -153,8 +153,6 @@ public:
 
     void dumpStats() const;
 
-    void registerDeletable(GCDeletable *d);
-
     size_t getUsedMem() const;
     size_t getAllocatedMem() const;
     size_t getLargeItemsMem() const;