QQmlNotifier: Always keep the isNotifying flag when updating senderPtr

When the sender gets deleted we still want to retain the flag that tells
us that the notifier is currently active. Otherwise we can miss the
error message about synchronously deleting objects while signal handlers
are in progress.

Task-number: QTBUG-73013
Change-Id: I8abba9b492327c15963d1875841c6822f345a89e
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

2 files changed, 53 insertions, 20 deletions

diff --git a/src/qml/qml/qqmlnotifier.cpp b/src/qml/qml/qqmlnotifier.cpp
index ac247ae0ad..0706b8c0cf 100644
--- a/src/qml/qml/qqmlnotifier.cpp
+++ b/src/qml/qml/qqmlnotifier.cpp
@@ -90,24 +90,20 @@ void QQmlNotifier::emitNotify(QQmlNotifierEndpoint *endpoint, void **a)
         NotifyListTraversalData &data = stack[i];
 
         if (!data.endpoint->isNotifying()) {
-            data.originalSenderPtr = data.endpoint->senderPtr;
+            data.endpoint->startNotifying(&data.originalSenderPtr);
             data.disconnectWatch = &data.originalSenderPtr;
-            data.endpoint->senderPtr = qintptr(data.disconnectWatch) | 0x1;
         } else {
             data.disconnectWatch = (qintptr *)(data.endpoint->senderPtr & ~0x1);
         }
     }
 
     while (--i >= 0) {
-        const NotifyListTraversalData &data = stack.at(i);
+        NotifyListTraversalData &data = stack[i];
         if (*data.disconnectWatch) {
-
             Q_ASSERT(QQmlNotifier_callbacks[data.endpoint->callback]);
             QQmlNotifier_callbacks[data.endpoint->callback](data.endpoint, a);
-
             if (data.disconnectWatch == &data.originalSenderPtr && data.originalSenderPtr) {
-                // End of notifying, restore values
-                data.endpoint->senderPtr = data.originalSenderPtr;
+                data.endpoint->stopNotifying(&data.originalSenderPtr);
             }
         }
     }
@@ -137,7 +133,7 @@ void QQmlNotifierEndpoint::connect(QObject *source, int sourceSignal, QQmlEngine
                qPrintable(engineName));
     }
 
-    senderPtr = qintptr(source);
+    setSender(qintptr(source));
     this->sourceSignal = sourceSignal;
     QQmlPropertyPrivate::flushSignal(source, sourceSignal);
     QQmlData *ddata = QQmlData::get(source, true);
diff --git a/src/qml/qml/qqmlnotifier_p.h b/src/qml/qml/qqmlnotifier_p.h
index d77e314de5..67d80a9d86 100644
--- a/src/qml/qml/qqmlnotifier_p.h
+++ b/src/qml/qml/qqmlnotifier_p.h
@@ -105,10 +105,16 @@ public:
     inline void disconnect();
 
     inline bool isNotifying() const;
+    inline void startNotifying(qintptr *originalSenderPtr);
+    inline void stopNotifying(qintptr *originalSenderPtr);
+
     inline void cancelNotify();
 
     inline int signalIndex() const { return sourceSignal; }
 
+    inline qintptr sender() const;
+    inline void setSender(qintptr sender);
+
     inline QObject *senderAsObject() const;
     inline QQmlNotifier *senderAsNotifier() const;
 
@@ -138,12 +144,9 @@ QQmlNotifier::~QQmlNotifier()
     while (endpoint) {
         QQmlNotifierEndpoint *n = endpoint;
         endpoint = n->next;
-
-        if (n->isNotifying()) *((qintptr *)(n->senderPtr & ~0x1)) = 0;
-
+        n->setSender(0x0);
         n->next = nullptr;
         n->prev = nullptr;
-        n->senderPtr = 0;
         n->sourceSignal = -1;
     }
     endpoints = nullptr;
@@ -193,7 +196,7 @@ void QQmlNotifierEndpoint::connect(QQmlNotifier *notifier)
     if (next) { next->prev = &next; }
     notifier->endpoints = this;
     prev = &notifier->endpoints;
-    senderPtr = qintptr(notifier);
+    setSender(qintptr(notifier));
 }
 
 void QQmlNotifierEndpoint::disconnect()
@@ -205,15 +208,15 @@ void QQmlNotifierEndpoint::disconnect()
 
     if (sourceSignal != -1) {
         QObject * const obj = senderAsObject();
+        Q_ASSERT(obj);
         QObjectPrivate * const priv = QObjectPrivate::get(obj);
         if (needsConnectNotify)
             priv->disconnectNotify(QMetaObjectPrivate::signal(obj->metaObject(), sourceSignal));
     }
 
-    if (isNotifying()) *((qintptr *)(senderPtr & ~0x1)) = 0;
+    setSender(0x0);
     next = nullptr;
     prev = nullptr;
-    senderPtr = 0;
     sourceSignal = -1;
 }
 
@@ -229,26 +232,60 @@ bool QQmlNotifierEndpoint::isNotifying() const
     return senderPtr & 0x1;
 }
 
+void QQmlNotifierEndpoint::startNotifying(qintptr *originalSenderPtr)
+{
+    Q_ASSERT(*originalSenderPtr == 0);
+    // Set the endpoint to notifying:
+    // - Save the original senderPtr,
+    *originalSenderPtr = senderPtr;
+    // - Take a pointer of it,
+    // - And assign that to the senderPtr, including a flag to signify "notifying".
+    senderPtr = qintptr(originalSenderPtr) | 0x1;
+}
+
+void QQmlNotifierEndpoint::stopNotifying(qintptr *originalSenderPtr)
+{
+    // End of notifying, restore values
+    Q_ASSERT((senderPtr & ~0x1) == qintptr(originalSenderPtr));
+    senderPtr = *originalSenderPtr;
+    *originalSenderPtr = 0;
+}
+
 /*!
 Cancel any notifies that are in progress.
 */
 void QQmlNotifierEndpoint::cancelNotify()
 {
     if (isNotifying()) {
-        qintptr sp = *((qintptr *)(senderPtr & ~0x1));
-        *((qintptr *)(senderPtr & ~0x1)) = 0;
-        senderPtr = sp;
+        auto *ptr = (qintptr *)(senderPtr & ~0x1);
+        Q_ASSERT(ptr);
+        senderPtr = *ptr;
+        *ptr = 0;
     }
 }
 
+qintptr QQmlNotifierEndpoint::sender() const
+{
+    return isNotifying() ? *(qintptr *)(senderPtr & ~0x1) : senderPtr;
+}
+
+void QQmlNotifierEndpoint::setSender(qintptr sender)
+{
+    // If we're just notifying, we write through to the originalSenderPtr
+    if (isNotifying())
+        *(qintptr *)(senderPtr & ~0x1) = sender;
+    else
+        senderPtr = sender;
+}
+
 QObject *QQmlNotifierEndpoint::senderAsObject() const
 {
-    return isNotifying()?((QObject *)(*((qintptr *)(senderPtr & ~0x1)))):((QObject *)senderPtr);
+    return (QObject *)(sender());
 }
 
 QQmlNotifier *QQmlNotifierEndpoint::senderAsNotifier() const
 {
-    return isNotifying()?((QQmlNotifier *)(*((qintptr *)(senderPtr & ~0x1)))):((QQmlNotifier *)senderPtr);
+    return (QQmlNotifier *)(sender());
 }
 
 QT_END_NAMESPACE