Binding: Avoid inconsistent state transition

When a QML element gets destroyed, it will at some point call
QQmlData::destroyed, which then sets addedToObject to false for all
bindings. It will also decrement the reference count of the bindings.
However, that doesn't necessarily result in the destruction of the
bindings, something else might still be referencing them.

Moerover, we also didn't disable the bindigs at this point. That means
that the bindings would still evaluate, even after the object has been
destroyed – and if the object is compleltely gone, QQmlData::wasDeleted
as used in QQmlBinding::update won't save us as we only have a pointer
to freed memory.

We avoid this whole mess by ensuring that a binding can't be in a state
where it's not attached to an object, but enabled.

This is based on changes from  bbf2c03226
"Add sticky bit to QQmlAbstractBinding", and therefore the actualy
change only needs to be added to older branches. We cherry-pick to dev
to get the test together with the context from the commit message.

Pick-to: 6.8 6.10 dev
Fixes: QTBUG-139306
Change-Id: I70d063a85c005afa9795d55095e5e834c88c1985
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

src/qml/qml/qqmlabstractbinding_p.h

@@ -68,9 +68,9 @@ public:
     inline QQmlAbstractBinding *nextBinding() const;
 
     inline bool canUseAccessor() const
-    { return m_nextBinding.tag().testFlag(CanUseAccessor); }
+    { return m_target.tag().testFlag(CanUseAccessor); }
     void setCanUseAccessor(bool canUseAccessor)
-    { m_nextBinding.setTag(m_nextBinding.tag().setFlag(CanUseAccessor, canUseAccessor)); }
+    { m_target.setTag(m_target.tag().setFlag(CanUseAccessor, canUseAccessor)); }
 
     struct RefCount {
         RefCount() {}
@@ -81,20 +81,21 @@ public:
     };
     RefCount ref;
 
+    // A binding can only be enabled if it's added to an object,
+    // and it can only be updating if it's enabled.
+    enum State {
+        Disabled        = 0,
+        AddedToObject   = 1,
+        BindingEnabled  = 2,
+        UpdatingBinding = 3,
+    };
+
     enum TargetTag {
         NoTargetTag     = 0x0,
-        UpdatingBinding = 0x1,
-        BindingEnabled  = 0x2
+        CanUseAccessor  = 0x1,
     };
     Q_DECLARE_FLAGS(TargetTags, TargetTag)
 
-    enum NextBindingTag {
-        NoBindingTag   = 0x0,
-        AddedToObject  = 0x1,
-        CanUseAccessor = 0x2
-    };
-    Q_DECLARE_FLAGS(NextBindingTags, NextBindingTag)
-
 protected:
     friend class QQmlData;
     friend class QQmlValueTypeProxyBinding;
@@ -120,20 +121,36 @@ protected:
     QTaggedPointer<QObject, TargetTags> m_target;
 
     // Pointer to the next binding in the linked list of bindings.
-    QTaggedPointer<QQmlAbstractBinding, NextBindingTags> m_nextBinding;
+    QTaggedPointer<QQmlAbstractBinding, State> m_nextBinding;
+
+private:
+    void setState(State state, bool enabled)
+    {
+        if (enabled) {
+            m_nextBinding.setTag(std::max(m_nextBinding.tag(), state));
+            return;
+        }
+
+        Q_ASSERT(state > 0);
+        m_nextBinding.setTag(std::min(m_nextBinding.tag(), State(state - 1)));
+    }
+
+    bool hasState(State state) const
+    {
+        return m_nextBinding.tag() >= state;
+    }
 };
 
 Q_DECLARE_OPERATORS_FOR_FLAGS(QQmlAbstractBinding::TargetTags)
-Q_DECLARE_OPERATORS_FOR_FLAGS(QQmlAbstractBinding::NextBindingTags)
 
 void QQmlAbstractBinding::setAddedToObject(bool v)
 {
-    m_nextBinding.setTag(m_nextBinding.tag().setFlag(AddedToObject, v));
+    setState(AddedToObject, v);
 }
 
 bool QQmlAbstractBinding::isAddedToObject() const
 {
-    return m_nextBinding.tag().testFlag(AddedToObject);
+    return hasState(AddedToObject);
 }
 
 QQmlAbstractBinding *QQmlAbstractBinding::nextBinding() const
@@ -152,22 +169,22 @@ void QQmlAbstractBinding::setNextBinding(QQmlAbstractBinding *b)
 
 bool QQmlAbstractBinding::updatingFlag() const
 {
-    return m_target.tag().testFlag(UpdatingBinding);
+    return hasState(UpdatingBinding);
 }
 
 void QQmlAbstractBinding::setUpdatingFlag(bool v)
 {
-    m_target.setTag(m_target.tag().setFlag(UpdatingBinding, v));
+    setState(UpdatingBinding, v);
 }
 
 bool QQmlAbstractBinding::enabledFlag() const
 {
-    return m_target.tag().testFlag(BindingEnabled);
+    return hasState(BindingEnabled);
 }
 
 void QQmlAbstractBinding::setEnabledFlag(bool v)
 {
-    m_target.setTag(m_target.tag().setFlag(BindingEnabled, v));
+    setState(BindingEnabled, v);
 }
 
 QT_END_NAMESPACE

tests/auto/qml/qqmlecmascript/data/bindingNoEvaluationIfObjectGone.qml

@@ -0,0 +1,9 @@
+import QtQml
+
+QtObject {
+    id: root
+    property int x: 42
+    property QtObject obj: QtObject {
+        property int y: root.x
+    }
+}

tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp

@@ -132,6 +132,7 @@ private slots:
     void exceptionClearsOnReeval();
     void exceptionSlotProducesWarning();
     void exceptionBindingProducesWarning();
+    void bindingNoEvaluationIfObjectGone();
     void compileInvalidBinding();
     void transientErrors();
     void shutdownErrors();
@@ -2535,6 +2536,27 @@ void tst_qqmlecmascript::exceptionBindingProducesWarning()
     QVERIFY(object != nullptr);
 }
 
+void tst_qqmlecmascript::bindingNoEvaluationIfObjectGone()
+{
+    QQmlEngine engine;
+    QQmlComponent component(&engine, testFileUrl("bindingNoEvaluationIfObjectGone.qml"));
+    std::unique_ptr<QObject> root(component.create());
+    QVERIFY(root);
+    QObject *targetObject = root->property("obj").value<QObject *>();
+    QVERIFY(targetObject);
+    QQmlProperty yProp(targetObject, u"y"_s, &engine);
+    // hold a reference to the binding to keep it alive
+    auto binding = QQmlAnyBinding::ofProperty(yProp);
+    QVERIFY(binding.asAbstractBinding());
+    QCOMPARE(binding.asAbstractBinding()->targetObject(), targetObject);
+    delete targetObject;
+    // the target object pointer of the binding hasn't been reset; we might
+    // want to change this in the future; that line can be then adapted
+    QVERIFY(binding.asAbstractBinding()->targetObject());
+    // trigger a binding reevaluation
+    root->setProperty("x", 10); // no ASAN warning/crash
+}
+
 void tst_qqmlecmascript::compileInvalidBinding()
 {
     // QTBUG-23387: ensure that invalid bindings don't cause a crash.