Delete weak JS objects on exit right away

This reduces memory leaks on exit when the engine is destroyed
after exec() has finished.

Change-Id: I917d103966d55b4dd3ba4e986ff902e29d8fb0ac
Reviewed-by: Chris Adams <christopher.adams@nokia.com>
Reviewed-by: Kent Hansen <kent.hansen@nokia.com>

diff --git a/src/qml/qml/v8/qv8qobjectwrapper.cpp b/src/qml/qml/v8/qv8qobjectwrapper.cpp
index ee80dde..d6e0190 100644 (file)
--- a/src/qml/qml/v8/qv8qobjectwrapper.cpp
+++ b/src/qml/qml/v8/qv8qobjectwrapper.cpp
@@ -1178,7 +1178,10 @@ bool QV8QObjectWrapper::deleteWeakQObject(QV8QObjectResource *resource, bool cal
                 if (ddata->ownContext && ddata->context)
                     ddata->context->emitDestruction();
                 ddata->isQueuedForDeletion = true;
-                object->deleteLater();
+                if (calledFromEngineDtor)
+                    delete object;
+                else
+                    object->deleteLater();
             }
         }
     }

diff --git a/tests/auto/qml/qqmlecmascript/data/jsOwnedObjectsDeletedOnEngineDestroy.qml b/tests/auto/qml/qqmlecmascript/data/jsOwnedObjectsDeletedOnEngineDestroy.qml
new file mode 100644 (file)
index 0000000..e1c4cc9
--- /dev/null
+++ b/tests/auto/qml/qqmlecmascript/data/jsOwnedObjectsDeletedOnEngineDestroy.qml
@@ -0,0 +1,6 @@
+import QtQuick 2.0
+
+Item {
+    property var jsOwnedObject1: deleteObject.object1()
+    property var jsOwnedObject2: deleteObject.object2
+}

diff --git a/tests/auto/qml/qqmlecmascript/testtypes.h b/tests/auto/qml/qqmlecmascript/testtypes.h
index a4615c5..70d4397 100644 (file)
--- a/tests/auto/qml/qqmlecmascript/testtypes.h
+++ b/tests/auto/qml/qqmlecmascript/testtypes.h
@@ -1459,21 +1459,29 @@ private:
 class MyDeleteObject : public QObject
 {
     Q_OBJECT
-    Q_PROPERTY(QObject *nestedObject READ nestedObject NOTIFY nestedObjectChanged);
-    Q_PROPERTY(int deleteNestedObject READ deleteNestedObject NOTIFY deleteNestedObjectChanged);
+    Q_PROPERTY(QObject *nestedObject READ nestedObject NOTIFY nestedObjectChanged)
+    Q_PROPERTY(int deleteNestedObject READ deleteNestedObject NOTIFY deleteNestedObjectChanged)
+    Q_PROPERTY(QObject *object2 READ object2 NOTIFY object2Changed)
 
 public:
-    MyDeleteObject() : m_nestedObject(new MyQmlObject) {}
+    MyDeleteObject() : m_nestedObject(new MyQmlObject), m_object1(0), m_object2(0) {}
 
+    Q_INVOKABLE QObject *object1() const { return m_object1; }
+    Q_INVOKABLE QObject *object2() const { return m_object2; }
+    void setObject1(QObject *object) { m_object1 = object; }
+    void setObject2(QObject *object) { m_object2 = object; emit object2Changed(); }
     QObject *nestedObject() const { return m_nestedObject; }
     int deleteNestedObject() { delete m_nestedObject; m_nestedObject = 0; return 1; }
 
 signals:
     void nestedObjectChanged();
     void deleteNestedObjectChanged();
+    void object2Changed();
 
 private:
     MyQmlObject *m_nestedObject;
+    QObject *m_object1;
+    QObject *m_object2;
 };
 
 class DateTimeExporter : public QObject

diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
index e5661bd..97fe099 100644 (file)
--- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
+++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
@@ -283,6 +283,7 @@ private slots:
     void fallbackBindings();
     void propertyOverride();
     void concatenatedStringPropertyAccess();
+    void jsOwnedObjectsDeletedOnEngineDestroy();
 
 private:
     static void propertyVarWeakRefCallback(v8::Persistent<v8::Value> object, void* parameter);
@@ -7294,6 +7295,38 @@ void tst_qqmlecmascript::concatenatedStringPropertyAccess()
     delete object;
 }
 
+void tst_qqmlecmascript::jsOwnedObjectsDeletedOnEngineDestroy()
+{
+    QQmlEngine *myEngine = new QQmlEngine;
+
+    MyDeleteObject deleteObject;
+    deleteObject.setObjectName("deleteObject");
+    QObject * const object1 = new QObject;
+    QObject * const object2 = new QObject;
+    object1->setObjectName("object1");
+    object2->setObjectName("object2");
+    deleteObject.setObject1(object1);
+    deleteObject.setObject2(object2);
+
+    // Objects returned by function calls get marked as destructible, but objects returned by
+    // property getters do not - therefore we explicitly set the object as destructible.
+    QQmlEngine::setObjectOwnership(object2, QQmlEngine::JavaScriptOwnership);
+
+    myEngine->rootContext()->setContextProperty("deleteObject", &deleteObject);
+    QQmlComponent component(myEngine, testFileUrl("jsOwnedObjectsDeletedOnEngineDestroy.qml"));
+    QObject *object = component.create();
+    QVERIFY(object);
+
+    // Destroying the engine should delete all JS owned QObjects
+    QSignalSpy spy1(object1, SIGNAL(destroyed()));
+    QSignalSpy spy2(object2, SIGNAL(destroyed()));
+    delete myEngine;
+    QCOMPARE(spy1.count(), 1);
+    QCOMPARE(spy2.count(), 1);
+
+    delete object;
+}
+
 QTEST_MAIN(tst_qqmlecmascript)
 
 #include "tst_qqmlecmascript.moc"