QProperty: Avoid spurious dependencies by suspending binding state

Avoid spurious bindings by resetting the binding state before calling
the setter of eager properties.

Fixes: QTBUG-88999
Change-Id: I1e3b5662307d906598335a21d306be9c606529d4
Reviewed-by: Lars Knoll <lars.knoll@qt.io>
(cherry picked from commit b21dba98e3e557eece0497aeea0f0beb70cc62da)
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
This commit is contained in:
Fabian Kosmale 2020-12-01 12:01:06 +01:00
parent 66dbe670da
commit ee0e17c73c
4 changed files with 36 additions and 10 deletions

View File

@ -303,7 +303,7 @@ BindingEvaluationState::BindingEvaluationState(QPropertyBindingPrivate *binding,
binding->clearDependencyObservers(); binding->clearDependencyObservers();
} }
CurrentCompatProperty::CurrentCompatProperty(QBindingStatus *status, QUntypedPropertyData *property) CompatPropertySafePoint::CompatPropertySafePoint(QBindingStatus *status, QUntypedPropertyData *property)
: property(property) : property(property)
{ {
// store a pointer to the currentBindingEvaluationState to avoid a TLS lookup in // store a pointer to the currentBindingEvaluationState to avoid a TLS lookup in
@ -311,6 +311,10 @@ CurrentCompatProperty::CurrentCompatProperty(QBindingStatus *status, QUntypedPro
currentState = &status->currentCompatProperty; currentState = &status->currentCompatProperty;
previousState = *currentState; previousState = *currentState;
*currentState = this; *currentState = this;
currentlyEvaluatingBindingList = &bindingStatus.currentlyEvaluatingBinding;
bindingState = *currentlyEvaluatingBindingList;
*currentlyEvaluatingBindingList = nullptr;
} }
QPropertyBindingPrivate *QPropertyBindingPrivate::currentlyEvaluatingBinding() QPropertyBindingPrivate *QPropertyBindingPrivate::currentlyEvaluatingBinding()
@ -1486,4 +1490,17 @@ QPropertyBindingData *QBindingStorage::bindingData_helper(QUntypedPropertyData *
return QBindingStoragePrivate(d).get(data, create); return QBindingStoragePrivate(d).get(data, create);
} }
BindingEvaluationState *suspendCurrentBindingStatus()
{
auto ret = bindingStatus.currentlyEvaluatingBinding;
bindingStatus.currentlyEvaluatingBinding = nullptr;
return ret;
}
void restoreBindingStatus(BindingEvaluationState *status)
{
bindingStatus.currentlyEvaluatingBinding = status;
}
QT_END_NAMESPACE QT_END_NAMESPACE

View File

@ -783,13 +783,13 @@ public:
namespace QtPrivate { namespace QtPrivate {
struct BindingEvaluationState; struct BindingEvaluationState;
struct CurrentCompatProperty; struct CompatPropertySafePoint;
} }
struct QBindingStatus struct QBindingStatus
{ {
QtPrivate::BindingEvaluationState *currentlyEvaluatingBinding = nullptr; QtPrivate::BindingEvaluationState *currentlyEvaluatingBinding = nullptr;
QtPrivate::CurrentCompatProperty *currentCompatProperty = nullptr; QtPrivate::CompatPropertySafePoint *currentCompatProperty = nullptr;
}; };
struct QBindingStorageData; struct QBindingStorageData;

View File

@ -134,16 +134,26 @@ struct BindingEvaluationState
BindingEvaluationState **currentState = nullptr; BindingEvaluationState **currentState = nullptr;
}; };
struct CurrentCompatProperty /*!
* \internal
* CompatPropertySafePoint needs to be constructed before the setter of
* a QObjectCompatProperty runs. It prevents spurious binding dependencies
* caused by reads of properties inside the compat property setter.
* Moreover, it ensures that we don't destroy bindings when using operator=
*/
struct CompatPropertySafePoint
{ {
Q_CORE_EXPORT CurrentCompatProperty(QBindingStatus *status, QUntypedPropertyData *property); Q_CORE_EXPORT CompatPropertySafePoint(QBindingStatus *status, QUntypedPropertyData *property);
~CurrentCompatProperty() ~CompatPropertySafePoint()
{ {
*currentState = previousState; *currentState = previousState;
*currentlyEvaluatingBindingList = bindingState;
} }
QUntypedPropertyData *property; QUntypedPropertyData *property;
CurrentCompatProperty *previousState = nullptr; CompatPropertySafePoint *previousState = nullptr;
CurrentCompatProperty **currentState = nullptr; CompatPropertySafePoint **currentState = nullptr;
QtPrivate::BindingEvaluationState **currentlyEvaluatingBindingList = nullptr;
QtPrivate::BindingEvaluationState *bindingState = nullptr;
}; };
} }
@ -360,7 +370,7 @@ class QObjectCompatProperty : public QPropertyData<T>
return false; return false;
// ensure value and setValue know we're currently evaluating our binding // ensure value and setValue know we're currently evaluating our binding
QBindingStorage *storage = qGetBindingStorage(thisData->owner()); QBindingStorage *storage = qGetBindingStorage(thisData->owner());
QtPrivate::CurrentCompatProperty guardThis(storage->bindingStatus, thisData); QtPrivate::CompatPropertySafePoint guardThis(storage->bindingStatus, thisData);
(thisData->owner()->*Setter)(copy.valueBypassingBindings()); (thisData->owner()->*Setter)(copy.valueBypassingBindings());
return true; return true;
} }

View File

@ -1401,7 +1401,6 @@ void tst_QProperty::noFakeDependencies()
QCOMPARE(slotCounter, 1); // sanity check QCOMPARE(slotCounter, 1); // sanity check
int old = bindingFunctionCalled; int old = bindingFunctionCalled;
fdc.setProp3(100); fdc.setProp3(100);
QEXPECT_FAIL("", "Known to create a spurious dependency", Continue);
QCOMPARE(old, bindingFunctionCalled); QCOMPARE(old, bindingFunctionCalled);
} }