QNotifiedProperty: avoid crash

We can end up in a situation where a (soon to be destroyed) observer is
owned by a binding which is about to be deleted. If in that situation
the binding is destroyed first, we end up with a dangling pointer
and ensuing memory corruption. Instead, we now first transfer the
ownership of the observer and only destroy the binding afterwards.

Fixes: QTBUG-85824
Change-Id: I721c0319281ada981ae7896bd2e02e9a0cc901b8
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

src/corelib/kernel/qproperty.cpp

@@ -338,10 +338,10 @@ void QPropertyBase::removeBinding()
 
     if (auto *existingBinding = d.bindingPtr()) {
         auto observer = existingBinding->takeObservers();
-        existingBinding->unlinkAndDeref();
         d_ptr &= ExtraBit;
         if (observer)
             d.setObservers(observer.ptr);
+        existingBinding->unlinkAndDeref();
     }
 }
 

tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp

@@ -73,6 +73,7 @@ private slots:
     void notifiedPropertyWithOldValueCallback();
     void notifiedPropertyWithGuard();
     void typeNoOperatorEqual();
+    void bindingValueReplacement();
 };
 
 void tst_QProperty::functorBinding()
@@ -1058,6 +1059,34 @@ void tst_QProperty::typeNoOperatorEqual()
     QVERIFY(u1.changedCalled);
 }
 
+
+struct Test {
+    void notify() {};
+    bool bindText(int);
+    bool bindIconText(int);
+    QProperty<int> text;
+    QNotifiedProperty<int, &Test::notify, &Test::bindIconText> iconText;
+};
+
+bool Test::bindIconText(int) {
+    Q_UNUSED(iconText.value()); // force read
+    if (!iconText.hasBinding()) {
+        iconText.setBinding(this, [=]() { return 0; });
+    }
+    return true;
+}
+
+void tst_QProperty::bindingValueReplacement()
+{
+    Test test;
+    test.text = 0;
+    test.bindIconText(0);
+    test.iconText.setValue(&test, 42); // should not crash
+    QCOMPARE(test.iconText.value(), 42);
+    test.text = 1;
+    QCOMPARE(test.iconText.value(), 42);
+}
+
 QTEST_MAIN(tst_QProperty);
 
 #include "tst_qproperty.moc"