From: Chris Adams <christopher.adams@nokia.com>
Date: Mon, 22 Aug 2011 05:04:01 +0000 (+1000)
Subject: Ensure JS-owned QObjects are cleaned up on v8engine dtor
X-Git-Url: http://git.silmor.de/gitweb/?a=commitdiff_plain;h=5d62a90fe129b09ecc6526ccd815469ef4ff774d;p=konrad%2Fqtdeclarative.git

Ensure JS-owned QObjects are cleaned up on v8engine dtor

This commit ensures that the garbage collector is invoked during
engine destruction.  This commit also adds a unit test which
ensures that the JS GC destroys JS-owned C++ objects correctly
when the QDeclarativeEngine is destroyed.

Task-number: QTBUG-20377
Change-Id: I2de1f2dfd1e60cc2f76abb523b99bf169d2a5a13
Reviewed-on: http://codereview.qt-project.org/3285
Reviewed-by: Qt Sanity Bot <qt_sanity_bot@ovi.com>
Reviewed-by: Aaron Kennedy <aaron.kennedy@nokia.com>
---

diff --git a/src/declarative/qml/qdeclarativeengine.cpp b/src/declarative/qml/qdeclarativeengine.cpp
index 1e35f0a..5d7710f 100644
--- a/src/declarative/qml/qdeclarativeengine.cpp
+++ b/src/declarative/qml/qdeclarativeengine.cpp
@@ -531,6 +531,9 @@ QDeclarativeEngine::~QDeclarativeEngine()
             d->moduleApiInstances.remove(key);
         }
     }
+
+    // ensure we clean up QObjects with JS ownership
+    d->v8engine()->gc();
 }
 
 /*! \fn void QDeclarativeEngine::quit()
diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/dynamicCreationOwnership.qml b/tests/auto/declarative/qdeclarativeecmascript/data/dynamicCreationOwnership.qml
new file mode 100644
index 0000000..ed396d4
--- /dev/null
+++ b/tests/auto/declarative/qdeclarativeecmascript/data/dynamicCreationOwnership.qml
@@ -0,0 +1,20 @@
+import QtQuick 2.0
+import Qt.test 1.0
+
+Item {
+    id: obj
+    objectName: "obj"
+
+    MyDynamicCreationDestructionObject {
+        id: mdcdo
+        objectName: "mdcdo"
+    }
+
+    function dynamicallyCreateJsOwnedObject() {
+        mdcdo.createNew();
+    }
+
+    function performGc() {
+        gc();
+    }
+}
diff --git a/tests/auto/declarative/qdeclarativeecmascript/testtypes.cpp b/tests/auto/declarative/qdeclarativeecmascript/testtypes.cpp
index 0057c11..ac4cb30 100644
--- a/tests/auto/declarative/qdeclarativeecmascript/testtypes.cpp
+++ b/tests/auto/declarative/qdeclarativeecmascript/testtypes.cpp
@@ -192,6 +192,8 @@ void registerTypes()
 
     qmlRegisterType<CircularReferenceObject>("Qt.test", 1, 0, "CircularReferenceObject");
     qmlRegisterType<CircularReferenceHandle>("Qt.test", 1, 0, "CircularReferenceHandle");
+
+    qmlRegisterType<MyDynamicCreationDestructionObject>("Qt.test", 1, 0, "MyDynamicCreationDestructionObject");
 }
 
 #include "testtypes.moc"
diff --git a/tests/auto/declarative/qdeclarativeecmascript/testtypes.h b/tests/auto/declarative/qdeclarativeecmascript/testtypes.h
index ebe8cc5..3fd6eaf 100644
--- a/tests/auto/declarative/qdeclarativeecmascript/testtypes.h
+++ b/tests/auto/declarative/qdeclarativeecmascript/testtypes.h
@@ -1072,6 +1072,47 @@ private:
 };
 Q_DECLARE_METATYPE(CircularReferenceHandle*)
 
+class MyDynamicCreationDestructionObject : public QObject
+{
+    Q_OBJECT
+    Q_PROPERTY (int intProperty READ intProperty WRITE setIntProperty NOTIFY intPropertyChanged)
+
+public:
+    MyDynamicCreationDestructionObject(QObject *parent = 0) : QObject(parent), m_intProperty(0), m_dtorCount(0)
+    {
+    }
+
+    ~MyDynamicCreationDestructionObject()
+    {
+        if (m_dtorCount) {
+            (*m_dtorCount)++;
+        }
+    }
+
+    int intProperty() const { return m_intProperty; }
+    void setIntProperty(int val) { m_intProperty = val; emit intPropertyChanged(); }
+
+    Q_INVOKABLE MyDynamicCreationDestructionObject *createNew()
+    {
+        // no parent == ownership transfers to JS; same dtor counter.
+        MyDynamicCreationDestructionObject *retn = new MyDynamicCreationDestructionObject;
+        retn->setDtorCount(m_dtorCount);
+        return retn;
+    }
+
+    void setDtorCount(int *dtorCount)
+    {
+        m_dtorCount = dtorCount;
+    }
+
+signals:
+    void intPropertyChanged();
+
+private:
+    int m_intProperty;
+    int *m_dtorCount;
+};
+
 void registerTypes();
 
 #endif // TESTTYPES_H
diff --git a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
index 0d88449..db0f52c 100644
--- a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
+++ b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
@@ -164,6 +164,7 @@ private slots:
     void bug1();
     void bug2();
     void dynamicCreationCrash();
+    void dynamicCreationOwnership();
     void regExpBug();
     void nullObjectBinding();
     void deletedEngine();
@@ -1664,6 +1665,40 @@ void tst_qdeclarativeecmascript::dynamicCreationCrash()
     delete object;
 }
 
+// ownership transferred to JS, ensure that GC runs the dtor
+void tst_qdeclarativeecmascript::dynamicCreationOwnership()
+{
+    int dtorCount = 0;
+    int expectedDtorCount = 1; // start at 1 since we expect mdcdo to dtor too.
+
+    // allow the engine to go out of scope too.
+    {
+        QDeclarativeEngine dcoEngine;
+        QDeclarativeComponent component(&dcoEngine, TEST_FILE("dynamicCreationOwnership.qml"));
+        QObject *object = component.create();
+        QVERIFY(object != 0);
+        MyDynamicCreationDestructionObject *mdcdo = object->findChild<MyDynamicCreationDestructionObject*>("mdcdo");
+        QVERIFY(mdcdo != 0);
+        mdcdo->setDtorCount(&dtorCount);
+
+        for (int i = 1; i < 105; ++i, ++expectedDtorCount) {
+            QMetaObject::invokeMethod(object, "dynamicallyCreateJsOwnedObject");
+            if (i % 90 == 0) {
+                // we do this once manually, but it should be done automatically
+                // when the engine goes out of scope (since it should gc in dtor)
+                QMetaObject::invokeMethod(object, "performGc");
+            }
+            if (i % 10 == 0) {
+                QCoreApplication::processEvents(QEventLoop::DeferredDeletion);
+            }
+        }
+
+        delete object;
+    }
+    QCoreApplication::processEvents(QEventLoop::DeferredDeletion);
+    QCOMPARE(dtorCount, expectedDtorCount);
+}
+
 //QTBUG-9367
 void tst_qdeclarativeecmascript::regExpBug()
 {