QProperty: Do not involve semi-destroyed QObjects in bindings
Once we're in ~QObject, only methods of QObject are still valid. Notably, no setter of any derived class is still valid. Thus, to be safe we must no longer react to binding changes of those properties. To ensure that this happens for QObjectCompatProperty properties, we explicitly clear the binding storage. Fixes a particles3d example crash. Change-Id: I10d2bfa5e96621ce039d751cffaf3ac41893623e Reviewed-by: Laszlo Agocs <laszlo.agocs@qt.io> Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
This commit is contained in:
parent
0ebe5c9ef6
commit
f6fb118c94
@ -943,6 +943,11 @@ QObject::QObject(QObjectPrivate &dd, QObject *parent)
|
|||||||
Q_TRACE(QObject_ctor, this);
|
Q_TRACE(QObject_ctor, this);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void QObjectPrivate::clearBindingStorage()
|
||||||
|
{
|
||||||
|
bindingStorage.clear();
|
||||||
|
}
|
||||||
|
|
||||||
/*!
|
/*!
|
||||||
Destroys the object, deleting all its child objects.
|
Destroys the object, deleting all its child objects.
|
||||||
|
|
||||||
@ -973,6 +978,10 @@ QObject::~QObject()
|
|||||||
d->wasDeleted = true;
|
d->wasDeleted = true;
|
||||||
d->blockSig = 0; // unblock signals so we always emit destroyed()
|
d->blockSig = 0; // unblock signals so we always emit destroyed()
|
||||||
|
|
||||||
|
// If we reached this point, we need to clear the binding data
|
||||||
|
// as the corresponding properties are no longer useful
|
||||||
|
d->clearBindingStorage();
|
||||||
|
|
||||||
QtSharedPointer::ExternalRefCountData *sharedRefcount = d->sharedRefcount.loadRelaxed();
|
QtSharedPointer::ExternalRefCountData *sharedRefcount = d->sharedRefcount.loadRelaxed();
|
||||||
if (sharedRefcount) {
|
if (sharedRefcount) {
|
||||||
if (sharedRefcount->strongref.loadRelaxed() > 0) {
|
if (sharedRefcount->strongref.loadRelaxed() > 0) {
|
||||||
|
@ -326,6 +326,8 @@ public:
|
|||||||
QObjectPrivate(int version = QObjectPrivateVersion);
|
QObjectPrivate(int version = QObjectPrivateVersion);
|
||||||
virtual ~QObjectPrivate();
|
virtual ~QObjectPrivate();
|
||||||
void deleteChildren();
|
void deleteChildren();
|
||||||
|
// used to clear binding storage early in ~QObject
|
||||||
|
void clearBindingStorage();
|
||||||
|
|
||||||
inline void checkForIncompatibleLibraryVersion(int version) const;
|
inline void checkForIncompatibleLibraryVersion(int version) const;
|
||||||
|
|
||||||
|
@ -2138,6 +2138,12 @@ QBindingStorage::~QBindingStorage()
|
|||||||
QBindingStoragePrivate(d).destroy();
|
QBindingStoragePrivate(d).destroy();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void QBindingStorage::clear()
|
||||||
|
{
|
||||||
|
QBindingStoragePrivate(d).destroy();
|
||||||
|
d = nullptr;
|
||||||
|
}
|
||||||
|
|
||||||
// ### Unused, retained for BC with 6.0
|
// ### Unused, retained for BC with 6.0
|
||||||
void QBindingStorage::maybeUpdateBindingAndRegister_helper(const QUntypedPropertyData *data) const
|
void QBindingStorage::maybeUpdateBindingAndRegister_helper(const QUntypedPropertyData *data) const
|
||||||
{
|
{
|
||||||
|
@ -945,6 +945,7 @@ class Q_CORE_EXPORT QBindingStorage
|
|||||||
|
|
||||||
template<typename Class, typename T, auto Offset, auto Setter, auto Signal>
|
template<typename Class, typename T, auto Offset, auto Setter, auto Signal>
|
||||||
friend class QObjectCompatProperty;
|
friend class QObjectCompatProperty;
|
||||||
|
friend class QObjectPrivate;
|
||||||
public:
|
public:
|
||||||
QBindingStorage();
|
QBindingStorage();
|
||||||
~QBindingStorage();
|
~QBindingStorage();
|
||||||
@ -973,6 +974,7 @@ public:
|
|||||||
return bindingData_helper(data, create);
|
return bindingData_helper(data, create);
|
||||||
}
|
}
|
||||||
private:
|
private:
|
||||||
|
void clear();
|
||||||
void registerDependency_helper(const QUntypedPropertyData *data) const;
|
void registerDependency_helper(const QUntypedPropertyData *data) const;
|
||||||
// ### Unused, but keep for BC
|
// ### Unused, but keep for BC
|
||||||
void maybeUpdateBindingAndRegister_helper(const QUntypedPropertyData *data) const;
|
void maybeUpdateBindingAndRegister_helper(const QUntypedPropertyData *data) const;
|
||||||
|
@ -57,6 +57,7 @@ private slots:
|
|||||||
void basicDependencies();
|
void basicDependencies();
|
||||||
void multipleDependencies();
|
void multipleDependencies();
|
||||||
void bindingWithDeletedDependency();
|
void bindingWithDeletedDependency();
|
||||||
|
void dependencyChangeDuringDestruction();
|
||||||
void recursiveDependency();
|
void recursiveDependency();
|
||||||
void bindingAfterUse();
|
void bindingAfterUse();
|
||||||
void bindingFunctionDtorCalled();
|
void bindingFunctionDtorCalled();
|
||||||
@ -199,6 +200,35 @@ void tst_QProperty::bindingWithDeletedDependency()
|
|||||||
QCOMPARE(propertySelector.value(), staticProperty.value());
|
QCOMPARE(propertySelector.value(), staticProperty.value());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
class ChangeDuringDtorTester : public QObject
|
||||||
|
{
|
||||||
|
Q_OBJECT
|
||||||
|
Q_PROPERTY(int prop READ prop WRITE setProp BINDABLE bindableProp)
|
||||||
|
|
||||||
|
public:
|
||||||
|
void setProp(int i) { m_prop = i;}
|
||||||
|
int prop() const { return m_prop; }
|
||||||
|
QBindable<int> bindableProp() { return &m_prop; }
|
||||||
|
private:
|
||||||
|
Q_OBJECT_COMPAT_PROPERTY(ChangeDuringDtorTester, int, m_prop, &ChangeDuringDtorTester::setProp)
|
||||||
|
};
|
||||||
|
|
||||||
|
void tst_QProperty::dependencyChangeDuringDestruction()
|
||||||
|
{
|
||||||
|
auto tester = std::make_unique<ChangeDuringDtorTester>();
|
||||||
|
QProperty<int> iprop {42};
|
||||||
|
tester->bindableProp().setBinding(Qt::makePropertyBinding(iprop));
|
||||||
|
QObject::connect(tester.get(), &QObject::destroyed, [&](){
|
||||||
|
iprop = 12;
|
||||||
|
});
|
||||||
|
bool failed = false;
|
||||||
|
auto handler = tester->bindableProp().onValueChanged([&](){
|
||||||
|
failed = true;
|
||||||
|
});
|
||||||
|
tester.reset();
|
||||||
|
QVERIFY(!failed);
|
||||||
|
}
|
||||||
|
|
||||||
void tst_QProperty::recursiveDependency()
|
void tst_QProperty::recursiveDependency()
|
||||||
{
|
{
|
||||||
QProperty<int> first(1);
|
QProperty<int> first(1);
|
||||||
|
Loading…
x
Reference in New Issue
Block a user