Fix tst_qqmlecmascript::jsOwnedObjectsDeletedOnEngineDestroy

When destructing the JS engine and deleting the JS wrappers, we may decide
to delete the QObjects of JS owned JS QObject wrappers. During this phase
(engine destruction) we need to delete the QObject instantly instead of
deferred (the default when _running_ the engine).

* Delete the memory manager first in the ExecutionEngine destructor, as
  some of the user code destructors we may end up calling still expect
  a somewhat functioning engine unfortunately.
* Fixed also some bugs in Persistent/WeakValue: When assing a value,
  detach properly if refcount > 0
* Fixed a bug when clearing a weak value during sweep, when we wouldn't
  pick up the correct next pointer for a weak value that didn't have the mark
  bit set

Change-Id: I960e58ecbb40b0ecf2bc07fe661a1ec706fcca61
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

7 files changed, 65 insertions, 43 deletions

diff --git a/src/qml/qml/qqmlvmemetaobject.cpp b/src/qml/qml/qqmlvmemetaobject.cpp
index ace9ff9a65..09edd3af89 100644
--- a/src/qml/qml/qqmlvmemetaobject.cpp
+++ b/src/qml/qml/qqmlvmemetaobject.cpp
@@ -73,7 +73,8 @@ void QQmlVMEVariantQObjectPtr::objectDestroyed(QObject *)
         if (m_isVar && m_target->varPropertiesInitialized && !m_target->varProperties.isEmpty()) {
             // Set the var property to NULL
             QV4::ArrayObject *a = m_target->varProperties.value().asArrayObject();
-            a->putIndexed(m_index - m_target->firstVarPropertyIndex, QV4::Value::nullValue());
+            if (a)
+                a->putIndexed(m_index - m_target->firstVarPropertyIndex, QV4::Value::nullValue());
         }
 
         m_target->activate(m_target->object, m_target->methodOffset() + m_index, 0);
diff --git a/src/qml/qml/v4/qv4engine.cpp b/src/qml/qml/v4/qv4engine.cpp
index f4f80a4a1b..81bd9417a6 100644
--- a/src/qml/qml/v4/qv4engine.cpp
+++ b/src/qml/qml/v4/qv4engine.cpp
@@ -269,13 +269,13 @@ ExecutionEngine::ExecutionEngine(QQmlJS::EvalISelFactory *factory)
 
 ExecutionEngine::~ExecutionEngine()
 {
+    delete memoryManager;
     emptyClass->destroy();
     delete identifierCache;
     delete bumperPointerAllocator;
     delete regExpCache;
     UnwindHelper::deregisterFunctions(functions);
     qDeleteAll(functions);
-    delete memoryManager;
     delete regExpAllocator;
     delete executableAllocator;
 }
diff --git a/src/qml/qml/v4/qv4mm.cpp b/src/qml/qml/v4/qv4mm.cpp
index fcc660e3db..cf37c695f3 100644
--- a/src/qml/qml/v4/qv4mm.cpp
+++ b/src/qml/qml/v4/qv4mm.cpp
@@ -333,7 +333,9 @@ std::size_t MemoryManager::sweep()
         if (Managed *m = weak->value.asManaged()) {
             if (!m->markBit) {
                 weak->value = Value::emptyValue();
+                PersistentValuePrivate *n = weak->next;
                 weak->removeFromList();
+                weak = n;
                 continue;
             }
         }
@@ -448,6 +450,18 @@ void MemoryManager::setEnableGC(bool enableGC)
 
 MemoryManager::~MemoryManager()
 {
+    PersistentValuePrivate *weak = m_weakValues;
+    while (weak) {
+        if (QObjectWrapper *qobjectWrapper = weak->value.asQObjectWrapper()) {
+            weak->ref();
+            qobjectWrapper->deleteQObject(/*deleteInstantly*/ true);
+            PersistentValuePrivate *n = weak->next;
+            weak->deref();
+            weak = n;
+        } else
+            weak = weak->next;
+    }
+
     PersistentValuePrivate *persistent = m_persistentValues;
     while (persistent) {
         PersistentValuePrivate *n = persistent->next;
diff --git a/src/qml/qml/v4/qv4value.cpp b/src/qml/qml/v4/qv4value.cpp
index a6f5beb99f..fb463b5abd 100644
--- a/src/qml/qml/v4/qv4value.cpp
+++ b/src/qml/qml/v4/qv4value.cpp
@@ -248,6 +248,8 @@ PersistentValue &PersistentValue::operator=(const PersistentValue &other)
     d = other.d;
     if (d)
         d->ref();
+
+    return *this;
 }
 
 PersistentValue &PersistentValue::operator =(const Value &other)
@@ -256,26 +258,8 @@ PersistentValue &PersistentValue::operator =(const Value &other)
         d = new PersistentValuePrivate(other);
         return *this;
     }
-    d->value = other;
-    Managed *m = d->value.asManaged();
-    if (!d->prev) {
-        if (m) {
-            ExecutionEngine *engine = m->engine();
-            if (engine) {
-                d->prev = &engine->memoryManager->m_persistentValues;
-                d->next = engine->memoryManager->m_persistentValues;
-                *d->prev = d;
-                if (d->next)
-                    d->next->prev = &d->next;
-            }
-        }
-    } else if (!m) {
-        if (d->next)
-            d->next->prev = d->prev;
-        *d->prev = d->next;
-        d->prev = 0;
-        d->next = 0;
-    }
+    d = d->detach(other);
+    return *this;
 }
 
 PersistentValue::~PersistentValue()
@@ -308,6 +292,8 @@ WeakValue &WeakValue::operator=(const WeakValue &other)
     d = other.d;
     if (d)
         d->ref();
+
+    return *this;
 }
 
 WeakValue &WeakValue::operator =(const Value &other)
@@ -316,26 +302,8 @@ WeakValue &WeakValue::operator =(const Value &other)
         d = new PersistentValuePrivate(other, /*weak*/true);
         return *this;
     }
-    d->value = other;
-    Managed *m = d->value.asManaged();
-    if (!d->prev) {
-        if (m) {
-            ExecutionEngine *engine = m->engine();
-            if (engine) {
-                d->prev = &engine->memoryManager->m_weakValues;
-                d->next = engine->memoryManager->m_weakValues;
-                *d->prev = d;
-                if (d->next)
-                    d->next->prev = &d->next;
-            }
-        }
-    } else if (!m) {
-        if (d->next)
-            d->next->prev = d->prev;
-        *d->prev = d->next;
-        d->prev = 0;
-        d->next = 0;
-    }
+    d = d->detach(other, /*weak*/true);
+    return *this;
 }
 
 
@@ -383,6 +351,7 @@ void PersistentValuePrivate::removeFromList()
         if (next)
             next->prev = prev;
         *prev = next;
+        next = 0;
         prev = 0;
     }
 }
@@ -397,3 +366,30 @@ void PersistentValuePrivate::deref()
     }
 }
 
+PersistentValuePrivate *PersistentValuePrivate::detach(const QV4::Value &value, bool weak)
+{
+    if (refcount == 1) {
+        this->value = value;
+
+        Managed *m = value.asManaged();
+        if (!prev) {
+            if (m) {
+                ExecutionEngine *engine = m->engine();
+                if (engine) {
+                    PersistentValuePrivate **listRoot = weak ? &engine->memoryManager->m_weakValues : &engine->memoryManager->m_persistentValues;
+                    prev = listRoot;
+                    next = *listRoot;
+                    *prev = this;
+                    if (next)
+                        next->prev = &this->next;
+                }
+            }
+        } else if (!m)
+            removeFromList();
+
+        return this;
+    }
+    --refcount;
+    return new PersistentValuePrivate(value, weak);
+}
+
diff --git a/src/qml/qml/v4/qv4value_p.h b/src/qml/qml/v4/qv4value_p.h
index 9914467c39..45604740e6 100644
--- a/src/qml/qml/v4/qv4value_p.h
+++ b/src/qml/qml/v4/qv4value_p.h
@@ -577,6 +577,7 @@ struct PersistentValuePrivate
     void removeFromList();
     void ref() { ++refcount; }
     void deref();
+    PersistentValuePrivate *detach(const QV4::Value &value, bool weak = false);
 };
 
 class Q_QML_EXPORT PersistentValue
diff --git a/src/qml/qml/v8/qv8qobjectwrapper.cpp b/src/qml/qml/v8/qv8qobjectwrapper.cpp
index 578eaea567..e1d5cab5c7 100644
--- a/src/qml/qml/v8/qv8qobjectwrapper.cpp
+++ b/src/qml/qml/v8/qv8qobjectwrapper.cpp
@@ -94,6 +94,11 @@ QObjectWrapper::QObjectWrapper(ExecutionEngine *engine, QObject *object)
 
 QObjectWrapper::~QObjectWrapper()
 {
+    deleteQObject();
+}
+
+void QObjectWrapper::deleteQObject(bool deleteInstantly)
+{
     if (!object)
         return;
     QQmlData *ddata = QQmlData::get(object, false);
@@ -104,7 +109,10 @@ QObjectWrapper::~QObjectWrapper()
         if (ddata->ownContext && ddata->context)
             ddata->context->emitDestruction();
         ddata->isQueuedForDeletion = true;
-        object->deleteLater();
+        if (deleteInstantly)
+            delete object;
+        else
+            object->deleteLater();
     }
 }
 
diff --git a/src/qml/qml/v8/qv8qobjectwrapper_p.h b/src/qml/qml/v8/qv8qobjectwrapper_p.h
index b9d528e3c7..1811889873 100644
--- a/src/qml/qml/v8/qv8qobjectwrapper_p.h
+++ b/src/qml/qml/v8/qv8qobjectwrapper_p.h
@@ -87,6 +87,8 @@ struct Q_QML_EXPORT QObjectWrapper : public QV4::Object
     QV8Engine *v8Engine; // ### Remove again.
     QQmlGuard<QObject> object;
 
+    void deleteQObject(bool deleteInstantly = false);
+
 private:
     String *m_destroy;
     String *m_toString;