ScrollBar: fix crash on exit when flickable created before ScrollView

In the case from the related QTBUG-106118, the ListView is declared
before the ScrollView, so when the QObject destruction happens, it gets
destroyed first. The ScrollView being destroyed causes the attached
ScrollBar object to be destroyed, which causes the attached object to
access the flickable (ListView) which has already been destroyed. So a
workaround for the crash is to declare the ListView after the
ScrollView.

With typical usage where ScrollView is used as-is, ScrollView (and
hence QQuickScrollBarAttached) is destroyed before the flickable.

This patch also adds a PRINT_LISTENERS debugging helper, as item
change listener crashes can be difficult to debug.

Fixes: QTBUG-106106
Change-Id: I8d0d5544ea28c1f1557abdc38d48b26d21a7f1f5
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>
(cherry picked from commit c6f51533d9d2aadd3441b3292b6ab5435c35a03b)

3 files changed, 45 insertions, 0 deletions

diff --git a/src/quick/items/qquickitem.cpp b/src/quick/items/qquickitem.cpp
index a0fa0e6fa3..563f69b748 100644
--- a/src/quick/items/qquickitem.cpp
+++ b/src/quick/items/qquickitem.cpp
@@ -61,6 +61,7 @@ Q_DECLARE_LOGGING_CATEGORY(lcPtr)
 Q_DECLARE_LOGGING_CATEGORY(lcTransient)
 Q_LOGGING_CATEGORY(lcHandlerParent, "qt.quick.handler.parent")
 Q_LOGGING_CATEGORY(lcVP, "qt.quick.viewport")
+Q_LOGGING_CATEGORY(lcChangeListeners, "qt.quick.item.changelisteners")
 
 // after 100ms, a mouse/non-mouse cursor conflict is resolved in favor of the mouse handler
 static const quint64 kCursorOverrideTimeout = 100;
@@ -3895,9 +3896,23 @@ void QQuickItem::updatePolish()
 {
 }
 
+#define PRINT_LISTENERS() \
+do { \
+    qDebug().nospace() << q_func() << " (" << this \
+        << ") now has the following listeners:"; \
+    for (const auto listener : changeListeners) { \
+        const auto objectPrivate = dynamic_cast<QObjectPrivate*>(listener.listener); \
+        qDebug().nospace() << "- " << listener << " (QObject: " << (objectPrivate ? objectPrivate->q_func() : nullptr) << ")"; \
+    } \
+} \
+while (false)
+
 void QQuickItemPrivate::addItemChangeListener(QQuickItemChangeListener *listener, ChangeTypes types)
 {
     changeListeners.append(ChangeListener(listener, types));
+
+    if (lcChangeListeners().isDebugEnabled())
+        PRINT_LISTENERS();
 }
 
 void QQuickItemPrivate::updateOrAddItemChangeListener(QQuickItemChangeListener *listener, ChangeTypes types)
@@ -3908,12 +3923,18 @@ void QQuickItemPrivate::updateOrAddItemChangeListener(QQuickItemChangeListener *
         changeListeners[index].types = changeListener.types;
     else
         changeListeners.append(changeListener);
+
+    if (lcChangeListeners().isDebugEnabled())
+        PRINT_LISTENERS();
 }
 
 void QQuickItemPrivate::removeItemChangeListener(QQuickItemChangeListener *listener, ChangeTypes types)
 {
     ChangeListener change(listener, types);
     changeListeners.removeOne(change);
+
+    if (lcChangeListeners().isDebugEnabled())
+        PRINT_LISTENERS();
 }
 
 void QQuickItemPrivate::updateOrAddGeometryChangeListener(QQuickItemChangeListener *listener,
@@ -3925,6 +3946,9 @@ void QQuickItemPrivate::updateOrAddGeometryChangeListener(QQuickItemChangeListen
         changeListeners[index].gTypes = change.gTypes;  //we may have different GeometryChangeTypes
     else
         changeListeners.append(change);
+
+    if (lcChangeListeners().isDebugEnabled())
+        PRINT_LISTENERS();
 }
 
 void QQuickItemPrivate::updateOrRemoveGeometryChangeListener(QQuickItemChangeListener *listener,
@@ -3938,6 +3962,9 @@ void QQuickItemPrivate::updateOrRemoveGeometryChangeListener(QQuickItemChangeLis
         if (index > -1)
             changeListeners[index].gTypes = change.gTypes;  //we may have different GeometryChangeTypes
     }
+
+    if (lcChangeListeners().isDebugEnabled())
+        PRINT_LISTENERS();
 }
 
 /*!
@@ -9648,6 +9675,13 @@ quint64 QQuickItemPrivate::_q_createJSWrapper(QV4::ExecutionEngine *engine)
     return (engine->memoryManager->allocate<QQuickItemWrapper>(q_func()))->asReturnedValue();
 }
 
+QDebug operator<<(QDebug debug, const QQuickItemPrivate::ChangeListener &listener)
+{
+   QDebugStateSaver stateSaver(debug);
+   debug.nospace() << "ChangeListener listener=" << listener.listener << " types=" << listener.types;
+   return debug;
+}
+
 QT_END_NAMESPACE
 
 #include <moc_qquickitem.cpp>
diff --git a/src/quick/items/qquickitem_p.h b/src/quick/items/qquickitem_p.h
index 5437aeb46c..428878c45d 100644
--- a/src/quick/items/qquickitem_p.h
+++ b/src/quick/items/qquickitem_p.h
@@ -323,6 +323,11 @@ public:
         QQuickItemChangeListener *listener;
         ChangeTypes types;
         QQuickGeometryChange gTypes;  //NOTE: not used for ==
+
+#ifndef QT_NO_DEBUG_STREAM
+    private:
+        friend QDebug operator<<(QDebug debug, const QQuickItemPrivate::ChangeListener &listener);
+#endif // QT_NO_DEBUG_STREAM
     };
 
     // call QQuickItemChangeListener PMF
diff --git a/src/quicktemplates2/qquickscrollbar.cpp b/src/quicktemplates2/qquickscrollbar.cpp
index d861276e30..c17f3e773e 100644
--- a/src/quicktemplates2/qquickscrollbar.cpp
+++ b/src/quicktemplates2/qquickscrollbar.cpp
@@ -903,6 +903,7 @@ void QQuickScrollBarAttachedPrivate::setFlickable(QQuickFlickable *item)
         // The latter doesn't remove the listener but only resets its types. Thus, it leaves behind a dangling
         // pointer on destruction.
         QQuickItemPrivate::get(flickable)->removeItemChangeListener(this, QQuickItemPrivate::Geometry);
+        QQuickItemPrivate::get(flickable)->removeItemChangeListener(this, QQuickItemPrivate::Destroyed);
         if (horizontal)
             cleanupHorizontal();
         if (vertical)
@@ -912,7 +913,10 @@ void QQuickScrollBarAttachedPrivate::setFlickable(QQuickFlickable *item)
     flickable = item;
 
     if (item) {
+        // Don't know how to combine these calls into one, and as long as they're separate calls,
+        // the remove* calls above need to be separate too, otherwise they will have no effect.
         QQuickItemPrivate::get(item)->updateOrAddGeometryChangeListener(this, QQuickGeometryChange::Size);
+        QQuickItemPrivate::get(item)->updateOrAddItemChangeListener(this, QQuickItemPrivate::Destroyed);
         if (horizontal)
             initHorizontal();
         if (vertical)
@@ -1126,6 +1130,8 @@ void QQuickScrollBarAttachedPrivate::itemImplicitHeightChanged(QQuickItem *item)
 
 void QQuickScrollBarAttachedPrivate::itemDestroyed(QQuickItem *item)
 {
+    if (item == flickable)
+        flickable = nullptr;
     if (item == horizontal)
         horizontal = nullptr;
     if (item == vertical)