From 08e829e3a9dae0230678a8471275cebb4c8fb54e Mon Sep 17 00:00:00 2001 From: Chris Adams Date: Fri, 26 Aug 2011 13:54:00 +1000 Subject: Add garbage collector prologue callback to qv8engine This commit provides a generic way to manage persistent handles created by QML so that circular references don't cause leaks, by utilising v8's garbage collector callbacks. Change-Id: Ia898197fdf5d86b90915b835ce3e532f7d400de4 Reviewed-on: http://codereview.qt.nokia.com/3688 Reviewed-by: Qt Sanity Bot Reviewed-by: Aaron Kennedy --- .../data/handleReferenceManagement.handle.1.qml | 25 ++ .../data/handleReferenceManagement.handle.2.qml | 26 +++ .../data/handleReferenceManagement.object.1.qml | 31 +++ .../data/handleReferenceManagement.object.2.qml | 32 +++ .../qdeclarativeecmascript/testtypes.cpp | 3 + .../declarative/qdeclarativeecmascript/testtypes.h | 105 +++++++++ .../tst_qdeclarativeecmascript.cpp | 251 +++++++++++++++++++++ 7 files changed, 473 insertions(+) create mode 100644 tests/auto/declarative/qdeclarativeecmascript/data/handleReferenceManagement.handle.1.qml create mode 100644 tests/auto/declarative/qdeclarativeecmascript/data/handleReferenceManagement.handle.2.qml create mode 100644 tests/auto/declarative/qdeclarativeecmascript/data/handleReferenceManagement.object.1.qml create mode 100644 tests/auto/declarative/qdeclarativeecmascript/data/handleReferenceManagement.object.2.qml (limited to 'tests/auto/declarative/qdeclarativeecmascript') diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/handleReferenceManagement.handle.1.qml b/tests/auto/declarative/qdeclarativeecmascript/data/handleReferenceManagement.handle.1.qml new file mode 100644 index 0000000000..9c27653b34 --- /dev/null +++ b/tests/auto/declarative/qdeclarativeecmascript/data/handleReferenceManagement.handle.1.qml @@ -0,0 +1,25 @@ +import QtQuick 2.0 +import Qt.test 1.0 + +Item { + id: obj + objectName: "obj" + property CircularReferenceHandle first + property CircularReferenceHandle second + + CircularReferenceHandle { + id: crh + objectName: "crh" + } + + function createReference() { + first = crh.generate(crh); + second = crh.generate(crh); + // NOTE: manually add reference from first to second + // in unit test prior reparenting and gc. + } + + function performGc() { + gc(); + } +} diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/handleReferenceManagement.handle.2.qml b/tests/auto/declarative/qdeclarativeecmascript/data/handleReferenceManagement.handle.2.qml new file mode 100644 index 0000000000..dc196263b4 --- /dev/null +++ b/tests/auto/declarative/qdeclarativeecmascript/data/handleReferenceManagement.handle.2.qml @@ -0,0 +1,26 @@ +import QtQuick 2.0 +import Qt.test 1.0 + +Item { + id: obj + objectName: "obj" + property CircularReferenceHandle first + property CircularReferenceHandle second + + CircularReferenceHandle { + id: crh + objectName: "crh" + } + + function circularReference() { + // generate the circularly referential pair + first = crh.generate(crh); + second = crh.generate(crh); + // note: must manually reparent in unit test + // after setting the handle references. + } + + function performGc() { + gc(); + } +} diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/handleReferenceManagement.object.1.qml b/tests/auto/declarative/qdeclarativeecmascript/data/handleReferenceManagement.object.1.qml new file mode 100644 index 0000000000..4fd1311c29 --- /dev/null +++ b/tests/auto/declarative/qdeclarativeecmascript/data/handleReferenceManagement.object.1.qml @@ -0,0 +1,31 @@ +import QtQuick 2.0 +import Qt.test 1.0 + +Item { + id: obj + objectName: "obj" + + property CircularReferenceObject first + property CircularReferenceObject second + + + CircularReferenceObject { + id: cro + objectName: "cro" + } + + function createReference() { + // generate the objects + first = cro.generate(cro); // has parent, so won't be collected + second = cro.generate(); // no parent, but will be kept alive by first's reference + first.addReference(second); + + // remove top level references + first = cro; + second = cro; + } + + function performGc() { + gc(); + } +} diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/handleReferenceManagement.object.2.qml b/tests/auto/declarative/qdeclarativeecmascript/data/handleReferenceManagement.object.2.qml new file mode 100644 index 0000000000..3f8415ce0f --- /dev/null +++ b/tests/auto/declarative/qdeclarativeecmascript/data/handleReferenceManagement.object.2.qml @@ -0,0 +1,32 @@ +import QtQuick 2.0 +import Qt.test 1.0 + +Item { + id: obj + objectName: "obj" + + property CircularReferenceObject first + property CircularReferenceObject second + + + CircularReferenceObject { + id: cro + objectName: "cro" + } + + function circularReference() { + // generate the circularly referential pair - they should still be collected + first = cro.generate(); // no parent, so should be collected + second = cro.generate(); // no parent, so should be collected + first.addReference(second); + second.addReference(first); + + // remove top level references + first = cro; + second = cro; + } + + function performGc() { + gc(); + } +} diff --git a/tests/auto/declarative/qdeclarativeecmascript/testtypes.cpp b/tests/auto/declarative/qdeclarativeecmascript/testtypes.cpp index 05d5510033..0057c117fa 100644 --- a/tests/auto/declarative/qdeclarativeecmascript/testtypes.cpp +++ b/tests/auto/declarative/qdeclarativeecmascript/testtypes.cpp @@ -189,6 +189,9 @@ void registerTypes() qRegisterMetaType("MyEnum2"); qRegisterMetaType("Qt::MouseButtons"); + + qmlRegisterType("Qt.test", 1, 0, "CircularReferenceObject"); + qmlRegisterType("Qt.test", 1, 0, "CircularReferenceHandle"); } #include "testtypes.moc" diff --git a/tests/auto/declarative/qdeclarativeecmascript/testtypes.h b/tests/auto/declarative/qdeclarativeecmascript/testtypes.h index 2738ee3d60..52b74affa8 100644 --- a/tests/auto/declarative/qdeclarativeecmascript/testtypes.h +++ b/tests/auto/declarative/qdeclarativeecmascript/testtypes.h @@ -57,6 +57,9 @@ #include #include +#include +#include + class MyQmlAttachedObject : public QObject { Q_OBJECT @@ -966,6 +969,108 @@ private: int m_methodCallCount; }; +class CircularReferenceObject : public QObject, + public QV8GCCallback::Node +{ + Q_OBJECT + +public: + CircularReferenceObject(QObject *parent = 0) + : QObject(parent), QV8GCCallback::Node(callback), m_referenced(0), m_dtorCount(0) + { + QV8GCCallback::addGcCallbackNode(this); + } + + ~CircularReferenceObject() + { + if (m_dtorCount) *m_dtorCount = *m_dtorCount + 1; + } + + Q_INVOKABLE void setDtorCount(int *dtorCount) + { + m_dtorCount = dtorCount; + } + + Q_INVOKABLE CircularReferenceObject *generate(QObject *parent = 0) + { + CircularReferenceObject *retn = new CircularReferenceObject(parent); + retn->m_dtorCount = m_dtorCount; + return retn; + } + + Q_INVOKABLE void addReference(QObject *other) + { + m_referenced = other; + } + + static void callback(QV8GCCallback::Referencer *r, QV8GCCallback::Node *n) + { + CircularReferenceObject *cro = static_cast(n); + if (cro->m_referenced) { + r->addRelationship(cro, cro->m_referenced); + } + } + +private: + QObject *m_referenced; + int *m_dtorCount; +}; +Q_DECLARE_METATYPE(CircularReferenceObject*) + +class CircularReferenceHandle : public QObject, + public QV8GCCallback::Node +{ + Q_OBJECT + +public: + CircularReferenceHandle(QObject *parent = 0) + : QObject(parent), QV8GCCallback::Node(gccallback), m_dtorCount(0) + { + QV8GCCallback::addGcCallbackNode(this); + } + + ~CircularReferenceHandle() + { + if (m_dtorCount) *m_dtorCount = *m_dtorCount + 1; + } + + Q_INVOKABLE void setDtorCount(int *dtorCount) + { + m_dtorCount = dtorCount; + } + + Q_INVOKABLE CircularReferenceHandle *generate(QObject *parent = 0) + { + CircularReferenceHandle *retn = new CircularReferenceHandle(parent); + retn->m_dtorCount = m_dtorCount; + return retn; + } + + Q_INVOKABLE void addReference(v8::Persistent handle) + { + m_referenced = qPersistentNew(handle); + m_referenced.MakeWeak(static_cast(this), wrcallback); + } + + static void wrcallback(v8::Persistent handle, void *params) + { + CircularReferenceHandle *crh = static_cast(params); + qPersistentDispose(handle); + crh->m_referenced.Clear(); + } + + static void gccallback(QV8GCCallback::Referencer *r, QV8GCCallback::Node *n) + { + CircularReferenceHandle *crh = static_cast(n); + r->addRelationship(crh, crh->m_referenced); + } + +private: + v8::Persistent m_referenced; + int *m_dtorCount; +}; +Q_DECLARE_METATYPE(CircularReferenceHandle*) + void registerTypes(); #endif // TESTTYPES_H diff --git a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp index d04b38b67a..b732cd8193 100644 --- a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp +++ b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp @@ -153,6 +153,7 @@ private slots: void elementAssign(); void objectPassThroughSignals(); void booleanConversion(); + void handleReferenceManagement(); void bug1(); void bug2(); @@ -3210,6 +3211,256 @@ void tst_qdeclarativeecmascript::booleanConversion() delete object; } +void tst_qdeclarativeecmascript::handleReferenceManagement() +{ + + int dtorCount = 0; + { + // Linear QObject reference + QDeclarativeEngine hrmEngine; + QDeclarativeComponent component(&hrmEngine, TEST_FILE("handleReferenceManagement.object.1.qml")); + QObject *object = component.create(); + QVERIFY(object != 0); + CircularReferenceObject *cro = object->findChild("cro"); + cro->setDtorCount(&dtorCount); + QMetaObject::invokeMethod(object, "createReference"); + QMetaObject::invokeMethod(object, "performGc"); + QCoreApplication::processEvents(QEventLoop::DeferredDeletion); + QCOMPARE(dtorCount, 0); // second has JS ownership, kept alive by first's reference + delete object; + hrmEngine.collectGarbage(); + QCoreApplication::processEvents(QEventLoop::DeferredDeletion); + QCOMPARE(dtorCount, 3); + } + + dtorCount = 0; + { + // Circular QObject reference + QDeclarativeEngine hrmEngine; + QDeclarativeComponent component(&hrmEngine, TEST_FILE("handleReferenceManagement.object.2.qml")); + QObject *object = component.create(); + QVERIFY(object != 0); + CircularReferenceObject *cro = object->findChild("cro"); + cro->setDtorCount(&dtorCount); + QMetaObject::invokeMethod(object, "circularReference"); + QMetaObject::invokeMethod(object, "performGc"); + QCoreApplication::processEvents(QEventLoop::DeferredDeletion); + QCOMPARE(dtorCount, 2); // both should be cleaned up, since circular references shouldn't keep alive. + delete object; + hrmEngine.collectGarbage(); + QCoreApplication::processEvents(QEventLoop::DeferredDeletion); + QCOMPARE(dtorCount, 3); + } + + dtorCount = 0; + { + // Linear handle reference + QDeclarativeEngine hrmEngine; + QDeclarativeComponent component(&hrmEngine, TEST_FILE("handleReferenceManagement.handle.1.qml")); + QObject *object = component.create(); + QVERIFY(object != 0); + CircularReferenceHandle *crh = object->findChild("crh"); + QVERIFY(crh != 0); + crh->setDtorCount(&dtorCount); + QMetaObject::invokeMethod(object, "createReference"); + CircularReferenceHandle *first = object->property("first").value(); + CircularReferenceHandle *second = object->property("second").value(); + QVERIFY(first != 0); + QVERIFY(second != 0); + first->addReference(QDeclarativeData::get(second)->v8object); // create reference + // now we have to reparent second and make second owned by JS. + second->setParent(0); + QDeclarativeEngine::setObjectOwnership(second, QDeclarativeEngine::JavaScriptOwnership); + QMetaObject::invokeMethod(object, "performGc"); + QCoreApplication::processEvents(QEventLoop::DeferredDeletion); + QCOMPARE(dtorCount, 0); // due to reference from first to second, second shouldn't be collected. + delete object; + hrmEngine.collectGarbage(); + QCoreApplication::processEvents(QEventLoop::DeferredDeletion); + QCOMPARE(dtorCount, 3); + } + + dtorCount = 0; + { + // Circular handle reference + QDeclarativeEngine hrmEngine; + QDeclarativeComponent component(&hrmEngine, TEST_FILE("handleReferenceManagement.handle.2.qml")); + QObject *object = component.create(); + QVERIFY(object != 0); + CircularReferenceHandle *crh = object->findChild("crh"); + QVERIFY(crh != 0); + crh->setDtorCount(&dtorCount); + QMetaObject::invokeMethod(object, "circularReference"); + CircularReferenceHandle *first = object->property("first").value(); + CircularReferenceHandle *second = object->property("second").value(); + QVERIFY(first != 0); + QVERIFY(second != 0); + first->addReference(QDeclarativeData::get(second)->v8object); // create circular reference + second->addReference(QDeclarativeData::get(first)->v8object); // note: must be weak. + // now we have to reparent and change ownership. + first->setParent(0); + second->setParent(0); + QDeclarativeEngine::setObjectOwnership(first, QDeclarativeEngine::JavaScriptOwnership); + QDeclarativeEngine::setObjectOwnership(second, QDeclarativeEngine::JavaScriptOwnership); + QMetaObject::invokeMethod(object, "performGc"); + QCoreApplication::processEvents(QEventLoop::DeferredDeletion); + QCOMPARE(dtorCount, 2); // despite circular references, both will be collected. + delete object; + hrmEngine.collectGarbage(); + QCoreApplication::processEvents(QEventLoop::DeferredDeletion); + QCOMPARE(dtorCount, 3); + } + + dtorCount = 0; + { + // multiple engine interaction - linear reference + QDeclarativeEngine hrmEngine1; + QDeclarativeEngine hrmEngine2; + QDeclarativeComponent component1(&hrmEngine1, TEST_FILE("handleReferenceManagement.handle.1.qml")); + QDeclarativeComponent component2(&hrmEngine2, TEST_FILE("handleReferenceManagement.handle.1.qml")); + QObject *object1 = component1.create(); + QObject *object2 = component2.create(); + QVERIFY(object1 != 0); + QVERIFY(object2 != 0); + CircularReferenceHandle *crh1 = object1->findChild("crh"); + CircularReferenceHandle *crh2 = object2->findChild("crh"); + QVERIFY(crh1 != 0); + QVERIFY(crh2 != 0); + crh1->setDtorCount(&dtorCount); + crh2->setDtorCount(&dtorCount); + QMetaObject::invokeMethod(object1, "createReference"); + QMetaObject::invokeMethod(object2, "createReference"); + CircularReferenceHandle *first1 = object1->property("first").value(); + CircularReferenceHandle *second1 = object1->property("second").value(); + CircularReferenceHandle *first2 = object2->property("first").value(); + CircularReferenceHandle *second2 = object2->property("second").value(); + QVERIFY(first1 != 0); + QVERIFY(second1 != 0); + QVERIFY(first2 != 0); + QVERIFY(second2 != 0); + first1->addReference(QDeclarativeData::get(second2)->v8object); // create reference across engines + // now we have to reparent second2 and make second2 owned by JS. + second2->setParent(0); + QDeclarativeEngine::setObjectOwnership(second2, QDeclarativeEngine::JavaScriptOwnership); + QMetaObject::invokeMethod(object1, "performGc"); + QMetaObject::invokeMethod(object2, "performGc"); + QCoreApplication::processEvents(QEventLoop::DeferredDeletion); + QCOMPARE(dtorCount, 0); // due to reference from first1 to second2, second2 shouldn't be collected. + delete object1; + delete object2; + hrmEngine1.collectGarbage(); + hrmEngine2.collectGarbage(); + QCoreApplication::processEvents(QEventLoop::DeferredDeletion); + QCOMPARE(dtorCount, 6); + } + + dtorCount = 0; + { + // multiple engine interaction - circular reference + QDeclarativeEngine hrmEngine1; + QDeclarativeEngine hrmEngine2; + QDeclarativeComponent component1(&hrmEngine1, TEST_FILE("handleReferenceManagement.handle.1.qml")); + QDeclarativeComponent component2(&hrmEngine2, TEST_FILE("handleReferenceManagement.handle.1.qml")); + QObject *object1 = component1.create(); + QObject *object2 = component2.create(); + QVERIFY(object1 != 0); + QVERIFY(object2 != 0); + CircularReferenceHandle *crh1 = object1->findChild("crh"); + CircularReferenceHandle *crh2 = object2->findChild("crh"); + QVERIFY(crh1 != 0); + QVERIFY(crh2 != 0); + crh1->setDtorCount(&dtorCount); + crh2->setDtorCount(&dtorCount); + QMetaObject::invokeMethod(object1, "createReference"); + QMetaObject::invokeMethod(object2, "createReference"); + CircularReferenceHandle *first1 = object1->property("first").value(); + CircularReferenceHandle *second1 = object1->property("second").value(); + CircularReferenceHandle *first2 = object2->property("first").value(); + CircularReferenceHandle *second2 = object2->property("second").value(); + QVERIFY(first1 != 0); + QVERIFY(second1 != 0); + QVERIFY(first2 != 0); + QVERIFY(second2 != 0); + first1->addReference(QDeclarativeData::get(second1)->v8object); // create linear reference within engine1 + second1->addReference(QDeclarativeData::get(second2)->v8object); // create linear reference across engines + second2->addReference(QDeclarativeData::get(first2)->v8object); // create linear reference within engine2 + first2->addReference(QDeclarativeData::get(first1)->v8object); // close the loop - circular ref across engines + // now we have to reparent and change ownership to JS. + first1->setParent(0); + second1->setParent(0); + first2->setParent(0); + second2->setParent(0); + QDeclarativeEngine::setObjectOwnership(first1, QDeclarativeEngine::JavaScriptOwnership); + QDeclarativeEngine::setObjectOwnership(second1, QDeclarativeEngine::JavaScriptOwnership); + QDeclarativeEngine::setObjectOwnership(first2, QDeclarativeEngine::JavaScriptOwnership); + QDeclarativeEngine::setObjectOwnership(second2, QDeclarativeEngine::JavaScriptOwnership); + QMetaObject::invokeMethod(object1, "performGc"); + QMetaObject::invokeMethod(object2, "performGc"); + QCoreApplication::processEvents(QEventLoop::DeferredDeletion); + QCOMPARE(dtorCount, 4); // circular references shouldn't keep them alive. + delete object1; + delete object2; + hrmEngine1.collectGarbage(); + hrmEngine2.collectGarbage(); + QCoreApplication::processEvents(QEventLoop::DeferredDeletion); + QCOMPARE(dtorCount, 6); + } + + dtorCount = 0; + { + // multiple engine interaction - linear reference with engine deletion + QDeclarativeEngine *hrmEngine1 = new QDeclarativeEngine; + QDeclarativeEngine *hrmEngine2 = new QDeclarativeEngine; + QDeclarativeComponent component1(hrmEngine1, TEST_FILE("handleReferenceManagement.handle.1.qml")); + QDeclarativeComponent component2(hrmEngine2, TEST_FILE("handleReferenceManagement.handle.1.qml")); + QObject *object1 = component1.create(); + QObject *object2 = component2.create(); + QVERIFY(object1 != 0); + QVERIFY(object2 != 0); + CircularReferenceHandle *crh1 = object1->findChild("crh"); + CircularReferenceHandle *crh2 = object2->findChild("crh"); + QVERIFY(crh1 != 0); + QVERIFY(crh2 != 0); + crh1->setDtorCount(&dtorCount); + crh2->setDtorCount(&dtorCount); + QMetaObject::invokeMethod(object1, "createReference"); + QMetaObject::invokeMethod(object2, "createReference"); + CircularReferenceHandle *first1 = object1->property("first").value(); + CircularReferenceHandle *second1 = object1->property("second").value(); + CircularReferenceHandle *first2 = object2->property("first").value(); + CircularReferenceHandle *second2 = object2->property("second").value(); + QVERIFY(first1 != 0); + QVERIFY(second1 != 0); + QVERIFY(first2 != 0); + QVERIFY(second2 != 0); + first1->addReference(QDeclarativeData::get(second1)->v8object); // create linear reference within engine1 + second1->addReference(QDeclarativeData::get(second2)->v8object); // create linear reference across engines + second2->addReference(QDeclarativeData::get(first2)->v8object); // create linear reference within engine2 + // now we have to reparent and change ownership to JS. + first1->setParent(crh1); + second1->setParent(0); + first2->setParent(0); + second2->setParent(0); + QDeclarativeEngine::setObjectOwnership(second1, QDeclarativeEngine::JavaScriptOwnership); + QDeclarativeEngine::setObjectOwnership(first2, QDeclarativeEngine::JavaScriptOwnership); + QDeclarativeEngine::setObjectOwnership(second2, QDeclarativeEngine::JavaScriptOwnership); + QMetaObject::invokeMethod(object1, "performGc"); + QMetaObject::invokeMethod(object2, "performGc"); + QCoreApplication::processEvents(QEventLoop::DeferredDeletion); + QCOMPARE(dtorCount, 0); + delete hrmEngine2; + QMetaObject::invokeMethod(object1, "performGc"); + QCoreApplication::processEvents(QEventLoop::DeferredDeletion); + QCOMPARE(dtorCount, 0); + delete object1; + delete object2; + hrmEngine1->collectGarbage(); + QCoreApplication::processEvents(QEventLoop::DeferredDeletion); + QCOMPARE(dtorCount, 6); + delete hrmEngine1; + } +} + // Test that assigning a null object works // Regressed with: df1788b4dbbb2826ae63f26bdf166342595343f4 void tst_qdeclarativeecmascript::nullObjectBinding() -- cgit v1.2.3