diff options
Diffstat (limited to 'src/corelib')
-rw-r--r-- | src/corelib/kernel/qproperty.cpp | 142 | ||||
-rw-r--r-- | src/corelib/kernel/qproperty.h | 11 | ||||
-rw-r--r-- | src/corelib/kernel/qproperty_p.h | 7 |
3 files changed, 135 insertions, 25 deletions
diff --git a/src/corelib/kernel/qproperty.cpp b/src/corelib/kernel/qproperty.cpp index 14ee881455..d25952c0bf 100644 --- a/src/corelib/kernel/qproperty.cpp +++ b/src/corelib/kernel/qproperty.cpp @@ -370,22 +370,27 @@ void QPropertyObserver::setSource(const QPropertyBindingData &property) d.observeProperty(propPrivate); } - QPropertyObserver::~QPropertyObserver() { + if (next.tag() == ActivelyExecuting) { + if (nodeState) + *nodeState = nullptr; + } QPropertyObserverPointer d{this}; d.unlink(); } QPropertyObserver::QPropertyObserver(QPropertyObserver &&other) { - std::swap(bindingToMarkDirty, other.bindingToMarkDirty); - std::swap(next, other.next); - std::swap(prev, other.prev); + bindingToMarkDirty = std::exchange(other.bindingToMarkDirty, {}); + next = std::exchange(other.next, {}); + prev = std::exchange(other.prev, {}); if (next) next->prev = &next; if (prev) prev.setPointer(this); + if (next.tag() == ActivelyExecuting) + *nodeState = this; } QPropertyObserver &QPropertyObserver::operator=(QPropertyObserver &&other) @@ -397,20 +402,22 @@ QPropertyObserver &QPropertyObserver::operator=(QPropertyObserver &&other) d.unlink(); bindingToMarkDirty = nullptr; - std::swap(bindingToMarkDirty, other.bindingToMarkDirty); - std::swap(next, other.next); - std::swap(prev, other.prev); + bindingToMarkDirty = std::exchange(other.bindingToMarkDirty, {}); + next = std::exchange(other.next, {}); + prev = std::exchange(other.prev, {}); if (next) next->prev = &next; if (prev) prev.setPointer(this); + if (next.tag() == ActivelyExecuting) + *nodeState = this; return *this; } void QPropertyObserverPointer::unlink() { - if (ptr->next.tag() & QPropertyObserver::ObserverNotifiesAlias) + if (ptr->next.tag() == QPropertyObserver::ObserverNotifiesAlias) ptr->aliasedPropertyData = nullptr; if (ptr->next) ptr->next->prev = ptr->prev; @@ -422,22 +429,81 @@ void QPropertyObserverPointer::unlink() void QPropertyObserverPointer::setChangeHandler(QPropertyObserver::ChangeHandler changeHandler) { + Q_ASSERT(ptr->next.tag() != QPropertyObserver::ActivelyExecuting); ptr->changeHandler = changeHandler; ptr->next.setTag(QPropertyObserver::ObserverNotifiesChangeHandler); } void QPropertyObserverPointer::setAliasedProperty(QUntypedPropertyData *property) { + Q_ASSERT(ptr->next.tag() != QPropertyObserver::ActivelyExecuting); ptr->aliasedPropertyData = property; ptr->next.setTag(QPropertyObserver::ObserverNotifiesAlias); } void QPropertyObserverPointer::setBindingToMarkDirty(QPropertyBindingPrivate *binding) { + Q_ASSERT(ptr->next.tag() != QPropertyObserver::ActivelyExecuting); ptr->bindingToMarkDirty = binding; ptr->next.setTag(QPropertyObserver::ObserverNotifiesBinding); } +/*! + \internal + QPropertyObserverNodeProtector is a RAII wrapper which takes care of the internal switching logic + for QPropertyObserverPointer::notify (described ibidem) +*/ +template <QPropertyObserver::ObserverTag tag> +struct QPropertyObserverNodeProtector { + QPropertyObserver m_placeHolder; + QPropertyObserver *&m_observer; + union { + QPropertyBindingPrivate *m_binding; + QPropertyObserver::ChangeHandler m_changeHandler; + }; + Q_REQUIRED_RESULT QPropertyObserverNodeProtector(QPropertyObserver *&observer) + : m_observer(observer) + { + static_assert(tag == QPropertyObserver::ObserverNotifiesBinding || + tag == QPropertyObserver::ObserverNotifiesChangeHandler); + if constexpr (tag == QPropertyObserver::ObserverNotifiesBinding) + m_binding = m_observer->bindingToMarkDirty; + else + m_changeHandler = m_observer->changeHandler; + switchNodes(m_placeHolder, m_observer); + m_observer->nodeState = &m_observer; + m_observer->next.setTag(QPropertyObserver::ActivelyExecuting); + m_placeHolder.next.setTag(QPropertyObserver::ActivelyExecuting); + } + + ~QPropertyObserverNodeProtector() { + if (m_observer) { + if constexpr (tag == QPropertyObserver::ObserverNotifiesBinding) + m_observer->bindingToMarkDirty = m_binding; + else + m_observer->changeHandler = m_changeHandler; + switchNodes(*m_observer, &m_placeHolder); + m_observer->next.setTag(tag); + } + // set tag to a safer value where we don't execute anything in the dtor + m_placeHolder.next.setTag(QPropertyObserver::ObserverNotifiesChangeHandler); + } + + /*! + \internal + replaces a node \a observer in the list with another node \a placeholder which must not be in the list + */ + static void switchNodes(QPropertyObserver &placeHolder, QPropertyObserver *observer) { + placeHolder.next = std::exchange(observer->next, {}); + placeHolder.prev = std::exchange(observer->prev, {}); + if (placeHolder.next) { + placeHolder.next->prev = &placeHolder.next; + } + if (placeHolder.prev) + placeHolder.prev.setPointer(&placeHolder); + }; +}; + /*! \internal \a propertyDataPtr is a pointer to the observed property's property data In case that property has a binding, \a triggeringBinding points to the binding's QPropertyBindingPrivate @@ -453,29 +519,62 @@ void QPropertyObserverPointer::notify(QPropertyBindingPrivate *triggeringBinding bool propertyChanged = true; auto observer = const_cast<QPropertyObserver*>(ptr); + /* + * The basic idea of the loop is as follows: We iterate over all observers in the linked list, + * and execute the functionality corresponding to their tag. + * However, complication arise due to the fact that the triggered operations might modify the list, + * which includes deletion and move of the current and next nodes. + * Therefore, we take a few safety precautions: + * 1. Before executing any action which might modify the list, we replace the actual node in the list with + * a placeholder node. As that one is stack allocated and owned by us, we can rest assured that it is + * still there after the action has executed, and placeHolder->next points to the actual next node in the list. + * Note that taking next at the beginning of the loop does not work, as the execuated action might either move + * or delete that node. + * 2. To properly handle deletion or moves of the real current node, we store a pointer to a pointer to itself in + * its nodeState. Whenever the node is reallocated and moved, we update that pointer to point to its new + * location. If the node is actually deleted, we set it to nullptr. + * 3. After the triggered action has finished, we can use that information to restore the list to contain the actual + * node again. We either switch the nodes with the real nodes current location, or, if the real node has been + * deleted, we simply unlink the temporary node. + */ while (observer) { - auto * const next = observer->next.data(); + QPropertyObserver *next = nullptr; + char preventBug[1] = {'\0'}; // QTBUG-87245 + Q_UNUSED(preventBug); switch (observer->next.tag()) { case QPropertyObserver::ObserverNotifiesChangeHandler: - if (!knownIfPropertyChanged && triggeringBinding) { - knownIfPropertyChanged = true; - - propertyChanged = triggeringBinding->evaluateIfDirtyAndReturnTrueIfValueChanged(propertyDataPtr); - } - if (!propertyChanged) - return; - - if (auto handlerToCall = std::exchange(observer->changeHandler, nullptr)) { + if (auto handlerToCall = observer->changeHandler) { + // both evaluateIfDirtyAndReturnTrueIfValueChanged and handlerToCall might modify the list + QPropertyObserverNodeProtector<QPropertyObserver::ObserverNotifiesChangeHandler> protector(observer); + if (!knownIfPropertyChanged && triggeringBinding) { + knownIfPropertyChanged = true; + propertyChanged = triggeringBinding->evaluateIfDirtyAndReturnTrueIfValueChanged(propertyDataPtr); + } + if (!propertyChanged) + return; handlerToCall(observer, propertyDataPtr); - observer->changeHandler = handlerToCall; + next = protector.m_placeHolder.next.data(); + } else { + next = observer->next.data(); } break; case QPropertyObserver::ObserverNotifiesBinding: - if (observer->bindingToMarkDirty) - observer->bindingToMarkDirty->markDirtyAndNotifyObservers(); + if (auto bindingToMarkDirty = observer->bindingToMarkDirty) { + QPropertyObserverNodeProtector<QPropertyObserver::ObserverNotifiesBinding> protector(observer); + bindingToMarkDirty->markDirtyAndNotifyObservers(); + next = protector.m_placeHolder.next.data(); + } else { + next = observer->next.data(); + } break; case QPropertyObserver::ObserverNotifiesAlias: + next = observer->next.data(); break; + case QPropertyObserver::ActivelyExecuting: + // recursion is already properly handled somewhere else + return; + default: + Q_UNREACHABLE(); } observer = next; } @@ -1484,5 +1583,4 @@ QPropertyBindingData *QBindingStorage::bindingData(QUntypedPropertyData *data, b return storage; } - QT_END_NAMESPACE diff --git a/src/corelib/kernel/qproperty.h b/src/corelib/kernel/qproperty.h index 88745d6cde..0d9a34dcde 100644 --- a/src/corelib/kernel/qproperty.h +++ b/src/corelib/kernel/qproperty.h @@ -221,9 +221,11 @@ class Q_CORE_EXPORT QPropertyObserver public: // Internal enum ObserverTag { - ObserverNotifiesBinding, - ObserverNotifiesChangeHandler, - ObserverNotifiesAlias, + ObserverNotifiesBinding, // observer was installed to notify bindings that obsverved property changed + ObserverNotifiesChangeHandler, // observer is a change handler, which runs on every change + ObserverNotifiesAlias, // used for QPropertyAlias + ActivelyExecuting // the observer is currently evaluated in QPropertyObserver::notifyObservers or its + // placeholder. We only can store 4 different values, therefore those two conflate }; QPropertyObserver() = default; @@ -257,6 +259,7 @@ private: QPropertyBindingPrivate *bindingToMarkDirty = nullptr; ChangeHandler changeHandler; QUntypedPropertyData *aliasedPropertyData; + QPropertyObserver **nodeState; }; QPropertyObserver(const QPropertyObserver &) = delete; @@ -265,6 +268,8 @@ private: friend struct QPropertyObserverPointer; friend struct QPropertyBindingDataPointer; friend class QPropertyBindingPrivate; + template<ObserverTag> + friend struct QPropertyObserverNodeProtector; }; template <typename Functor> diff --git a/src/corelib/kernel/qproperty_p.h b/src/corelib/kernel/qproperty_p.h index aa81aa9203..9185c939c6 100644 --- a/src/corelib/kernel/qproperty_p.h +++ b/src/corelib/kernel/qproperty_p.h @@ -241,6 +241,13 @@ public: void clearDependencyObservers() { for (size_t i = 0; i < qMin(dependencyObserverCount, inlineDependencyObservers.size()); ++i) { QPropertyObserverPointer p{&inlineDependencyObservers[i]}; + if (p.ptr->next.tag() == QPropertyObserver::ActivelyExecuting) { + *(p.ptr->nodeState) = nullptr; + p.ptr->nodeState = nullptr; + + // set tag to "safer" value, as we return the same observer pointer from allocateDependencyObserver + p.ptr->next.setTag(QPropertyObserver::ObserverNotifiesChangeHandler); + } p.unlink(); } if (heapObservers) |