From 90be89d771425044a84e9e79e4e668e065acc825 Mon Sep 17 00:00:00 2001 From: Andrei Golubev Date: Fri, 15 Jan 2021 16:46:59 +0100 Subject: Use new QObjectPrivate connection mechanism in dynamic connections Old API assumes sender == receiver, which results in wrong handling of connections when receiver is deleted: connection is not removed or notified elsehow as it's not really tied to a valid receiver Task-number: QTBUG-86368 Pick-to: 5.15 6.0 Change-Id: I0f3115f1b0f26cf353752ba2b8fd88e0f3bdd388 Reviewed-by: Ulf Hermann --- src/qml/jsruntime/qv4qobjectwrapper.cpp | 20 +++++++++-- .../data/scriptConnect.dynamic.1.qml | 32 +++++++++++++++++ .../data/scriptConnect.dynamic.2.qml | 33 ++++++++++++++++++ .../auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp | 40 ++++++++++++++++++++++ 4 files changed, 123 insertions(+), 2 deletions(-) create mode 100644 tests/auto/qml/qqmlecmascript/data/scriptConnect.dynamic.1.qml create mode 100644 tests/auto/qml/qqmlecmascript/data/scriptConnect.dynamic.2.qml diff --git a/src/qml/jsruntime/qv4qobjectwrapper.cpp b/src/qml/jsruntime/qv4qobjectwrapper.cpp index 08d0c6f40e..98d69b2f80 100644 --- a/src/qml/jsruntime/qv4qobjectwrapper.cpp +++ b/src/qml/jsruntime/qv4qobjectwrapper.cpp @@ -90,6 +90,7 @@ QT_BEGIN_NAMESPACE Q_LOGGING_CATEGORY(lcBindingRemoval, "qt.qml.binding.removal", QtWarningMsg) +Q_LOGGING_CATEGORY(lcObjectConnect, "qt.qml.object.connect", QtWarningMsg) // The code in this file does not violate strict aliasing, but GCC thinks it does // so turn off the warnings for us to have a clean build @@ -1100,7 +1101,16 @@ ReturnedValue QObjectWrapper::method_connect(const FunctionObject *b, const Valu QQmlPropertyPrivate::flushSignal(signalObject, propertyCache->methodIndexToSignalIndex(signalIndex)); } } - QObjectPrivate::connect(signalObject, signalIndex, slot, Qt::AutoConnection); + + QPair functionData = QObjectMethod::extractQtMethod(f); // align with disconnect + if (QObject *receiver = functionData.first) { + QObjectPrivate::connect(signalObject, signalIndex, receiver, slot, Qt::AutoConnection); + } else { + qCInfo(lcObjectConnect, + "Could not find receiver of the connection, using sender as receiver. Disconnect " + "explicitly (or delete the sender) to make sure the connection is removed."); + QObjectPrivate::connect(signalObject, signalIndex, signalObject, slot, Qt::AutoConnection); + } RETURN_UNDEFINED(); } @@ -1151,7 +1161,13 @@ ReturnedValue QObjectWrapper::method_disconnect(const FunctionObject *b, const V &functionData.second }; - QObjectPrivate::disconnect(signalObject, signalIndex, reinterpret_cast(&a)); + if (QObject *receiver = functionData.first) { + QObjectPrivate::disconnect(signalObject, signalIndex, receiver, + reinterpret_cast(&a)); + } else { + QObjectPrivate::disconnect(signalObject, signalIndex, signalObject, + reinterpret_cast(&a)); + } RETURN_UNDEFINED(); } diff --git a/tests/auto/qml/qqmlecmascript/data/scriptConnect.dynamic.1.qml b/tests/auto/qml/qqmlecmascript/data/scriptConnect.dynamic.1.qml new file mode 100644 index 0000000000..4814ff33f4 --- /dev/null +++ b/tests/auto/qml/qqmlecmascript/data/scriptConnect.dynamic.1.qml @@ -0,0 +1,32 @@ +import Qt.test 1.0 +import QtQuick 2.0 +import "scriptConnect.6.js" as Script + +Item { + id: root + + property int test: 0 + property var dynamicObjectProperty: undefined + + signal outer + Component { + id: comp + Item { + signal inner + onInner: { Script.testFunction(); root.destroyDynamicObject(); } + } + } + + function destroyDynamicObject() { + if (dynamicObjectProperty) { + dynamicObjectProperty.destroy(); + dynamicObjectProperty = undefined; + gc(); + } + } + + Component.onCompleted: { + dynamicObjectProperty = comp.createObject(root); + root.outer.connect(dynamicObjectProperty.inner); + } +} diff --git a/tests/auto/qml/qqmlecmascript/data/scriptConnect.dynamic.2.qml b/tests/auto/qml/qqmlecmascript/data/scriptConnect.dynamic.2.qml new file mode 100644 index 0000000000..9f1f67bbeb --- /dev/null +++ b/tests/auto/qml/qqmlecmascript/data/scriptConnect.dynamic.2.qml @@ -0,0 +1,33 @@ +import Qt.test 1.0 +import QtQuick 2.0 +import "scriptConnect.6.js" as Script + +Item { + id: root + + property int test: 0 + property var dynamicObjectProperty: undefined + + signal outer + Component { + id: comp + Item { + signal inner + onInner: { Script.testFunction(); root.disconnectAndDestroyDynamicObject(); } + } + } + + function disconnectAndDestroyDynamicObject() { + if (dynamicObjectProperty) { + root.outer.disconnect(dynamicObjectProperty.inner); + dynamicObjectProperty.destroy(); + dynamicObjectProperty = undefined; + gc(); + } + } + + Component.onCompleted: { + dynamicObjectProperty = comp.createObject(root); + root.outer.connect(dynamicObjectProperty.inner); + } +} diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp index 13b6ad7700..02d044aa63 100644 --- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp +++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp @@ -3500,6 +3500,46 @@ void tst_qqmlecmascript::scriptConnect() delete object; } + + { + QQmlComponent component(&engine, testFileUrl("scriptConnect.dynamic.1.qml")); + + QObject *object = component.create(); + QVERIFY(object != nullptr); + + QCOMPARE(object->property("test").toInt(), 0); + + QMetaObject::invokeMethod(object, "outer"); + QCOMPARE(object->property("test").toInt(), 1); + + // process the dynamic object deletion queried with deleteLater() + QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete); + QCoreApplication::processEvents(); + + // after deletion, further invocations do not update the property + QMetaObject::invokeMethod(object, "outer"); + QCOMPARE(object->property("test").toInt(), 1); + + delete object; + } + + { + QQmlComponent component(&engine, testFileUrl("scriptConnect.dynamic.2.qml")); + + QObject *object = component.create(); + QVERIFY(object != nullptr); + + QCOMPARE(object->property("test").toInt(), 0); + QMetaObject::invokeMethod(object, "outer"); + QCOMPARE(object->property("test").toInt(), 1); + + // no need to manually process events here, as we disconnect explicitly + + QMetaObject::invokeMethod(object, "outer"); + QCOMPARE(object->property("test").toInt(), 1); + + delete object; + } } void tst_qqmlecmascript::scriptDisconnect() -- cgit v1.2.3