diff options
author | Ulf Hermann <ulf.hermann@qt.io> | 2023-01-02 21:48:12 +0100 |
---|---|---|
committer | Qt Cherry-pick Bot <cherrypick_bot@qt-project.org> | 2023-01-05 12:14:57 +0000 |
commit | eebb73c2c834537b923f8d23a81e557ab6baa2f7 (patch) | |
tree | f007b9db35126b2c0a175f0ac7be6d8a3e689bf1 | |
parent | e1134503fac0ab5522c7ca13c22cba8b30702a30 (diff) |
QQmlObjectCreator: Do not crash on read-only bindable
If the binding was not actually set (because the bindable is readonly)
then it's dead after the pop_front. We cannot examine it anymore, and we
don't have to.
Fixes: QTBUG-109597
Change-Id: I3bf0ca501aa9ad45a64ad181b685ca6d9d325231
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
(cherry picked from commit 4dfcaa7ee8273914ea8cd9fd232064ce95cb15d1)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
-rw-r--r-- | src/qml/qml/qqmlobjectcreator.cpp | 24 | ||||
-rw-r--r-- | tests/auto/qml/qqmlecmascript/data/readOnlyBindable.qml | 7 | ||||
-rw-r--r-- | tests/auto/qml/qqmlecmascript/testtypes.cpp | 1 | ||||
-rw-r--r-- | tests/auto/qml/qqmlecmascript/testtypes.h | 19 | ||||
-rw-r--r-- | tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp | 18 |
5 files changed, 61 insertions, 8 deletions
diff --git a/src/qml/qml/qqmlobjectcreator.cpp b/src/qml/qml/qqmlobjectcreator.cpp index 05951edfbc..64d1f79044 100644 --- a/src/qml/qml/qqmlobjectcreator.cpp +++ b/src/qml/qml/qqmlobjectcreator.cpp @@ -1408,16 +1408,24 @@ bool QQmlObjectCreator::finalize(QQmlInstantiationInterrupt &interrupt) void *argv[] = { &bindable }; // allow interception target->metaObject()->metacall(target, QMetaObject::BindableProperty, index, argv); - bindable.setBinding(qmlBinding); + const bool success = bindable.setBinding(qmlBinding); + + // Only pop_front after setting the binding as the bindings are refcounted. sharedState->allQPropertyBindings.pop_front(); - if (auto priv = QPropertyBindingPrivate::get(qmlBinding); priv->hasCustomVTable()) { - auto qmlBindingPriv = static_cast<QQmlPropertyBinding *>(priv); - auto jsExpression = qmlBindingPriv->jsExpression(); - const bool canRemove = !qmlBinding.error().hasError() && !qmlBindingPriv->hasDependencies() - && !jsExpression->hasUnresolvedNames(); - if (canRemove) - bindable.takeBinding(); + + // If the binding was actually not set, it's deleted now. + if (success) { + if (auto priv = QPropertyBindingPrivate::get(qmlBinding); priv->hasCustomVTable()) { + auto qmlBindingPriv = static_cast<QQmlPropertyBinding *>(priv); + auto jsExpression = qmlBindingPriv->jsExpression(); + const bool canRemove = !qmlBinding.error().hasError() + && !qmlBindingPriv->hasDependencies() + && !jsExpression->hasUnresolvedNames(); + if (canRemove) + bindable.takeBinding(); + } } + if (watcher.hasRecursed() || interrupt.shouldInterrupt()) return false; } diff --git a/tests/auto/qml/qqmlecmascript/data/readOnlyBindable.qml b/tests/auto/qml/qqmlecmascript/data/readOnlyBindable.qml new file mode 100644 index 0000000000..116036c9ff --- /dev/null +++ b/tests/auto/qml/qqmlecmascript/data/readOnlyBindable.qml @@ -0,0 +1,7 @@ +import Qt.test +import QtQuick + +ReadOnlyBindable { + property int v: 12 + x: v +} diff --git a/tests/auto/qml/qqmlecmascript/testtypes.cpp b/tests/auto/qml/qqmlecmascript/testtypes.cpp index 0b26979c4d..40f5e5cf5c 100644 --- a/tests/auto/qml/qqmlecmascript/testtypes.cpp +++ b/tests/auto/qml/qqmlecmascript/testtypes.cpp @@ -541,6 +541,7 @@ void registerTypes() qmlRegisterType<Receiver>("Qt.test", 1,0, "Receiver"); qmlRegisterType<Sender>("Qt.test", 1,0, "Sender"); + qmlRegisterTypesAndRevisions<ReadOnlyBindable>("Qt.test", 1); } #include "testtypes.moc" diff --git a/tests/auto/qml/qqmlecmascript/testtypes.h b/tests/auto/qml/qqmlecmascript/testtypes.h index e5c7ed0454..76c1a31e43 100644 --- a/tests/auto/qml/qqmlecmascript/testtypes.h +++ b/tests/auto/qml/qqmlecmascript/testtypes.h @@ -1978,6 +1978,25 @@ public slots: int slot1(int i, int j, int k) {return i+j+k;} }; +class ReadOnlyBindable : public QObject +{ + Q_OBJECT + QML_ELEMENT + Q_PROPERTY(int x READ x WRITE setX BINDABLE bindableX) + Q_OBJECT_BINDABLE_PROPERTY(ReadOnlyBindable, int, _xProp) + +public: + ReadOnlyBindable(QObject *parent = nullptr) + : QObject(parent) + { + setX(7); + } + + int x() const { return _xProp.value(); } + void setX(int x) { _xProp.setValue(x); } + QBindable<int> bindableX() const { return &_xProp; } +}; + void registerTypes(); #endif // TESTTYPES_H diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp index c1cbf79f90..7f48c244d8 100644 --- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp +++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp @@ -413,6 +413,8 @@ private slots: void internalClassParentGc(); void methodTypeMismatch(); + void doNotCrashOnReadOnlyBindable(); + private: // static void propertyVarWeakRefCallback(v8::Persistent<v8::Value> object, void* parameter); static void verifyContextLifetime(const QQmlRefPointer<QQmlContextData> &ctxt); @@ -10322,6 +10324,22 @@ void tst_qqmlecmascript::methodTypeMismatch() QCOMPARE(object->actuals(), QVariantList() << QVariant::fromValue((QObject *)nullptr)); } +void tst_qqmlecmascript::doNotCrashOnReadOnlyBindable() +{ + QQmlEngine engine; + QQmlComponent c(&engine, testFileUrl("readOnlyBindable.qml")); + QVERIFY2(c.isReady(), qPrintable(c.errorString())); +#ifndef QT_NO_DEBUG + QTest::ignoreMessage( + QtWarningMsg, + "setBinding: Could not set binding via bindable interface. " + "The QBindable is read-only."); +#endif + QScopedPointer<QObject> o(c.create()); + QVERIFY(o); + QCOMPARE(o->property("x").toInt(), 7); +} + QTEST_MAIN(tst_qqmlecmascript) #include "tst_qqmlecmascript.moc" |