Previously, only var property references could keep QObjects alive.
This meant that the garbage collector would collect QObject data
prematurely.
This commit ensures that variant properties keep QObjects alive as
required.
Task-number: QTBUG-24767
Change-Id: Ic98a06863251a3e7d6384ba9256810a78fb23406
Reviewed-by: Martin Jones <martin.jones@nokia.com>
aConnected.resize(metaData->aliasCount);
int list_type = qMetaTypeId<QQmlListProperty<QObject> >();
+ int qobject_type = qMetaTypeId<QObject*>();
+ int variant_type = qMetaTypeId<QVariant>();
+ bool needsGcCallback = (metaData->varPropertyCount > 0);
// ### Optimize
for (int ii = 0; ii < metaData->propertyCount - metaData->varPropertyCount; ++ii) {
if (t == list_type) {
listProperties.append(List(methodOffset() + ii, this));
data[ii].setValue(listProperties.count() - 1);
- }
+ } else if (!needsGcCallback && (t == qobject_type || t == variant_type)) {
+ needsGcCallback = true;
+ }
}
firstVarPropertyIndex = metaData->propertyCount - metaData->varPropertyCount;
- if (metaData->varPropertyCount)
+
+ // both var properties and variant properties can keep references to
+ // other QObjects, and var properties can also keep references to
+ // JavaScript objects. If we have any properties, we need to hook
+ // the gc() to ensure that references keep objects alive as needed.
+ if (needsGcCallback) {
QV8GCCallback::addGcCallbackNode(this);
+ }
}
QQmlVMEMetaObject::~QQmlVMEMetaObject()
{
QQmlVMEMetaObject *vmemo = static_cast<QQmlVMEMetaObject*>(node);
Q_ASSERT(vmemo);
- if (!vmemo->varPropertiesInitialized || vmemo->varProperties.IsEmpty() || !vmemo->ctxt || !vmemo->ctxt->engine)
+
+ if (!vmemo->ctxt || !vmemo->ctxt->engine)
return;
QQmlEnginePrivate *ep = QQmlEnginePrivate::get(vmemo->ctxt->engine);
+
+ // add references created by VMEVariant properties
+ int maxDataIdx = vmemo->metaData->propertyCount - vmemo->metaData->varPropertyCount;
+ for (int ii = 0; ii < maxDataIdx; ++ii) { // XXX TODO: optimise?
+ if (vmemo->data[ii].dataType() == QMetaType::QObjectStar) {
+ // possible QObject reference.
+ QObject *ref = vmemo->data[ii].asQObject();
+ if (ref) {
+ ep->v8engine()->addRelationshipForGC(vmemo->object, ref);
+ }
+ }
+ }
+
+ // add references created by var properties
+ if (!vmemo->varPropertiesInitialized || vmemo->varProperties.IsEmpty())
+ return;
ep->v8engine()->addRelationshipForGC(vmemo->object, vmemo->varProperties);
}
void QV8Engine::addRelationshipForGC(QObject *object, v8::Persistent<v8::Value> handle)
{
- if (handle.IsEmpty())
+ if (!object || handle.IsEmpty())
return;
bool handleShouldBeStrong = false;
void QV8Engine::addRelationshipForGC(QObject *object, QObject *other)
{
+ if (!object || !other)
+ return;
+
bool handleShouldBeStrong = false;
v8::Persistent<v8::Object> *implicitOwner = findOwnerAndStrength(object, &handleShouldBeStrong);
v8::Persistent<v8::Value> handle = QQmlData::get(other, true)->v8object;
- if (handleShouldBeStrong) {
+ if (handle.IsEmpty()) // no JS data to keep alive.
+ return;
+ else if (handleShouldBeStrong)
v8::V8::AddImplicitReferences(m_strongReferencer, &handle, 1);
- } else if (!implicitOwner->IsEmpty()) {
+ else if (!implicitOwner->IsEmpty())
v8::V8::AddImplicitReferences(*implicitOwner, &handle, 1);
- }
}
static QThreadStorage<QV8Engine::ThreadData*> perThreadEngineData;
--- /dev/null
+import QtQuick 2.0
+import Qt.test.qobjectApi 1.0 as QObjectApi
+
+Item {
+ property int variantCanary: 5
+ property var varCanary: 12
+
+ Component.onCompleted: {
+ QObjectApi.qobjectTestWritableProperty = 42;
+ }
+
+ Component.onDestruction: {
+ QObjectApi.qobjectTestWritableProperty = 43;
+ }
+}
--- /dev/null
+import QtQuick 2.0
+import Qt.test.qobjectApi 1.0 as QObjectApi
+
+Item {
+ property bool success: true
+ property Item testProp: null
+
+ // first we create an object and reference it via a dynamic variant property
+ function createReference() {
+ var c = Qt.createComponent("HRMDPComponent.qml");
+ testProp = c.createObject(null); // QML ownership.
+ }
+
+ // after a gc, it should not have been collected.
+ function ensureReference() {
+ if (testProp == null) success = false; // should not have triggered delete notify / zeroed testProp value
+ if (testProp.variantCanary != 5) success = false; // should not have deleted vmemo of object referenced by testProp
+ if (testProp.varCanary != 12) success = false; // should not have collected vmemo vmeProperties
+ if (QObjectApi.qobjectTestWritableProperty != 42) success = false; // should not have been set to 43.
+ }
+
+ // then we remove the reference.
+ function removeReference() {
+ testProp = null; // allow original object to be released.
+ }
+
+ // after a gc (and deferred deletion process) the object should be gone
+ function ensureDeletion() {
+ if (QObjectApi.qobjectTestWritableProperty != 43) success = false; // should have been set to 43.
+ }
+}
--- /dev/null
+import QtQuick 2.0
+import Qt.test.qobjectApi 1.0 as QObjectApi
+
+Item {
+ property bool success: true
+ property Item testProp: null
+
+ // first we create an object and reference it via a dynamic variant property
+ function createReference() {
+ var c = Qt.createComponent("HRMDPComponent.qml");
+ testProp = c.createObject(null); // QML ownership.
+ }
+
+ // after a gc, it should not have been collected.
+ function ensureReference() {
+ if (testProp == null) success = false; // should not have triggered delete notify / zeroed testProp value
+ if (testProp.variantCanary != 5) success = false; // should not have deleted vmemo of object referenced by testProp
+ if (testProp.varCanary != 12) success = false; // should not have collected vmemo vmeProperties
+ if (QObjectApi.qobjectTestWritableProperty != 42) success = false; // should not have been set to 43.
+ }
+
+ // then we manually delete the item being referenced
+ function manuallyDelete() {
+ QObjectApi.deleteQObject(testProp);
+ if (QObjectApi.qobjectTestWritableProperty != 43) success = false; // should have been set to 43.
+ }
+
+ // after a gc (and deferred deletion process) the object should be gone
+ function ensureDeleted() {
+ // a crash should not have occurred during the previous gc due to the
+ // VMEMO attempting to keep a previously deleted QObject alive.
+ if (testProp != null) success = false; // delete notify should have zeroed testProp value.
+ }
+}
--- /dev/null
+import QtQuick 2.0
+import Qt.test.qobjectApi 1.0 as QObjectApi
+
+Item {
+ property bool success: true
+ property variant testProp: null
+
+ // first we create an object and reference it via a dynamic variant property
+ function createReference() {
+ var c = Qt.createComponent("HRMDPComponent.qml");
+ testProp = c.createObject(null); // QML ownership.
+ }
+
+ // after a gc, it should not have been collected.
+ function ensureReference() {
+ if (testProp == null) success = false; // should not have triggered delete notify / zeroed testProp value
+ if (testProp.variantCanary != 5) success = false; // should not have deleted vmemo of object referenced by testProp
+ if (testProp.varCanary != 12) success = false; // should not have collected vmemo vmeProperties
+ if (QObjectApi.qobjectTestWritableProperty != 42) success = false; // should not have been set to 43.
+ }
+
+ // then we remove the reference.
+ function removeReference() {
+ testProp = null; // allow original object to be released.
+ }
+
+ // after a gc (and deferred deletion process) the object should be gone
+ function ensureDeletion() {
+ if (QObjectApi.qobjectTestWritableProperty != 43) success = false; // should have been set to 43.
+ }
+}
first = crh.generate(crh);
second = crh.generate(crh);
// note: must manually reparent in unit test
- // after setting the handle references.
+ // after setting the handle references, and
+ // then set the "first" and "second" property
+ // values to null (removing references from obj
+ // to the generated objects).
}
}
property bool success: false
property bool c1HasBeenDestroyed: false
- property Item c1 // not a js reference, so won't keep it alive
-
SignalEmittedComponent {
id: c2
- property int c1a: if (root.c1) root.c1.a; else 0; // will change during onDestruction handler of c1.
function c1aChangedHandler() {
// this should still be called, after c1 has been destroyed by gc,
// because the onDestruction handler of c1 will be triggered prior
// to when c1 will be invalidated.
- if (root.c1HasBeenDestroyed && c1a == 20) root.c1.setSuccessPropertyOf(root, true);
+ if (root.c1HasBeenDestroyed)
+ root.success = true;
+ // note: cannot call c1::setSuccessPropertyOf(root, true), since any
+ // reference to c1 would have kept c1 alive. So, set it directly.
}
}
// dynamically construct sibling. When it goes out of scope, it should be gc'd.
// note that the gc() will call weakqobjectcallback which will set queued for
// deletion flag -- thus QQmlData::wasDeleted() will return true for that object..
- var c = Qt.createComponent("SignalEmittedComponent.qml", root);
- var o = c.createObject(null); // JS ownership
- o.onAChanged.connect(c2.c1aChangedHandler);
- c1 = o;
- c1HasBeenDestroyed = true;
+ var comp = Qt.createComponent("SignalEmittedComponent.qml", root);
+ var c1 = comp.createObject(null); // JS ownership
+ c1.onAChanged.connect(c2.c1aChangedHandler);
+ c1HasBeenDestroyed = true; // gc will collect c1.
// return to event loop.
}
}
property bool success: false
property bool c1HasBeenDestroyed: false
- property Item c1 // not a js reference, so won't keep it alive
-
SignalEmittedComponent {
id: c2
- property int c1a: if (root.c1) root.c1.a; else 0; // will change during onDestruction handler of c1.
function c1aChangedHandler() {
// this should still be called, after c1 has been destroyed by gc,
// because the onDestruction handler of c1 will be triggered prior
// to when c1 will be invalidated.
- if (root.c1HasBeenDestroyed && c1a == 20) root.c1.setSuccessPropertyOf(root, true);
+ if (root.c1HasBeenDestroyed)
+ root.success = true;
}
}
// dynamically construct sibling. When it goes out of scope, it should be gc'd.
// note that the gc() will call weakqobjectcallback which will set queued for
// deletion flag -- thus QQmlData::wasDeleted() will return true for that object..
- var c = Qt.createComponent("SignalEmittedComponent.qml", root);
- var o = c.createObject(null); // JS ownership
- o.onAChanged.connect(c2.c1aChangedHandler);
- c1 = o;
- c1HasBeenDestroyed = true;
+ var comp = Qt.createComponent("SignalEmittedComponent.qml", root);
+ var c1 = comp.createObject(null); // JS ownership
+ c1.onAChanged.connect(c2.c1aChangedHandler);
+ c1HasBeenDestroyed = true; // gc will collect c1.
// return to event loop.
}
return ddata->indestructible?false:true;
}
+ Q_INVOKABLE void deleteQObject(QObject *obj) { delete obj; }
+
signals:
void qobjectTestPropertyChanged(int testProperty);
void qobjectTestWritablePropertyChanged(int testWritableProperty);
QVERIFY(second != 0);
first->addReference(QQmlData::get(second)->v8object); // create circular reference
second->addReference(QQmlData::get(first)->v8object); // note: must be weak.
- // now we have to reparent and change ownership.
+ // now we have to reparent and change ownership, and unset the property references.
first->setParent(0);
second->setParent(0);
QQmlEngine::setObjectOwnership(first, QQmlEngine::JavaScriptOwnership);
QQmlEngine::setObjectOwnership(second, QQmlEngine::JavaScriptOwnership);
+ object->setProperty("first", QVariant::fromValue<QObject*>(0));
+ object->setProperty("second", QVariant::fromValue<QObject*>(0));
gc(engine);
QCOMPARE(dtorCount, 2); // despite circular references, both will be collected.
delete object;
second1->addReference(QQmlData::get(second2)->v8object); // create linear reference across engines
second2->addReference(QQmlData::get(first2)->v8object); // create linear reference within engine2
first2->addReference(QQmlData::get(first1)->v8object); // close the loop - circular ref across engines
- // now we have to reparent and change ownership to JS.
+ // now we have to reparent and change ownership to JS, and remove property references.
first1->setParent(0);
second1->setParent(0);
first2->setParent(0);
QQmlEngine::setObjectOwnership(second1, QQmlEngine::JavaScriptOwnership);
QQmlEngine::setObjectOwnership(first2, QQmlEngine::JavaScriptOwnership);
QQmlEngine::setObjectOwnership(second2, QQmlEngine::JavaScriptOwnership);
+ object1->setProperty("first", QVariant::fromValue<QObject*>(0));
+ object1->setProperty("second", QVariant::fromValue<QObject*>(0));
+ object2->setProperty("first", QVariant::fromValue<QObject*>(0));
+ object2->setProperty("second", QVariant::fromValue<QObject*>(0));
gc(engine);
QCoreApplication::sendPostedEvents(0, QEvent::DeferredDelete);
QCoreApplication::processEvents();
QCoreApplication::processEvents();
QCOMPARE(dtorCount, 6); // all objects should have been cleaned up prior to deleting hrmEngine1.
}
+
+ {
+ // Dynamic variant property reference keeps target alive
+ QQmlEngine hrmEngine;
+ QQmlComponent component(&hrmEngine, testFileUrl("handleReferenceManagement.dynprop.qml"));
+ QObject *object = component.create();
+ QVERIFY(object != 0);
+ QMetaObject::invokeMethod(object, "createReference");
+ gc(engine);
+ QMetaObject::invokeMethod(object, "ensureReference");
+ gc(engine);
+ QMetaObject::invokeMethod(object, "removeReference");
+ gc(engine);
+ QMetaObject::invokeMethod(object, "ensureDeletion");
+ QCOMPARE(object->property("success").toBool(), true);
+ delete object;
+ }
+
+ {
+ // Dynamic Item property reference keeps target alive
+ QQmlEngine hrmEngine;
+ QQmlComponent component(&hrmEngine, testFileUrl("handleReferenceManagement.dynprop.2.qml"));
+ QObject *object = component.create();
+ QVERIFY(object != 0);
+ QMetaObject::invokeMethod(object, "createReference");
+ gc(engine);
+ QMetaObject::invokeMethod(object, "ensureReference");
+ gc(engine);
+ QMetaObject::invokeMethod(object, "removeReference");
+ gc(engine);
+ QMetaObject::invokeMethod(object, "ensureDeletion");
+ QCOMPARE(object->property("success").toBool(), true);
+ delete object;
+ }
+
+ {
+ // Item property reference to deleted item doesn't crash
+ QQmlEngine hrmEngine;
+ QQmlComponent component(&hrmEngine, testFileUrl("handleReferenceManagement.dynprop.3.qml"));
+ QObject *object = component.create();
+ QVERIFY(object != 0);
+ QMetaObject::invokeMethod(object, "createReference");
+ gc(engine);
+ QMetaObject::invokeMethod(object, "ensureReference");
+ gc(engine);
+ QMetaObject::invokeMethod(object, "manuallyDelete");
+ gc(engine);
+ QMetaObject::invokeMethod(object, "ensureDeleted");
+ QCOMPARE(object->property("success").toBool(), true);
+ delete object;
+ }
}
void tst_qqmlecmascript::stringArg()
QVERIFY(obj != 0);
gc(engine); // should collect c1.
QMetaObject::invokeMethod(obj, "destroyC2");
- QTRY_VERIFY(obj->property("success").toBool());
+ QTRY_VERIFY(obj->property("success").toBool()); // handles events (incl. delete later).
delete obj;
}
}