From 17a34a1bab4f94a81bff3fb83e5f4bcfae2712dc Mon Sep 17 00:00:00 2001
From: Nyall Dawson <nyall.dawson@gmail.com>
Date: Tue, 3 Oct 2023 14:27:38 +1000
Subject: [PATCH] Avoid calling overridden virtual method in base class
 destructor

Fixes some undefined behavior when deleting layout items
---
 python/core/auto_generated/layout/qgslayoutframe.sip.in     | 1 +
 python/core/auto_generated/layout/qgslayoutitemgroup.sip.in | 1 +
 src/core/layout/qgslayout.cpp                               | 2 +-
 src/core/layout/qgslayoutframe.cpp                          | 6 ++++++
 src/core/layout/qgslayoutframe.h                            | 1 +
 src/core/layout/qgslayoutitem.cpp                           | 2 +-
 src/core/layout/qgslayoutitemgroup.cpp                      | 5 +++++
 src/core/layout/qgslayoutitemgroup.h                        | 1 +
 8 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/python/core/auto_generated/layout/qgslayoutframe.sip.in b/python/core/auto_generated/layout/qgslayoutframe.sip.in
index d3678dfa761f..fede2793085f 100644
--- a/python/core/auto_generated/layout/qgslayoutframe.sip.in
+++ b/python/core/auto_generated/layout/qgslayoutframe.sip.in
@@ -27,6 +27,7 @@ Base class for frame items, which form a layout multiframe item.
 Constructor for QgsLayoutFrame, with the specified parent ``layout``
 and belonging to a ``multiFrame``.
 %End
+    ~QgsLayoutFrame();
 
     static QgsLayoutFrame *create( QgsLayout *layout ) /Factory/;
 %Docstring
diff --git a/python/core/auto_generated/layout/qgslayoutitemgroup.sip.in b/python/core/auto_generated/layout/qgslayoutitemgroup.sip.in
index f8fa15316535..d513c6796318 100644
--- a/python/core/auto_generated/layout/qgslayoutitemgroup.sip.in
+++ b/python/core/auto_generated/layout/qgslayoutitemgroup.sip.in
@@ -25,6 +25,7 @@ A container for grouping several :py:class:`QgsLayoutItems`.
 %Docstring
 Constructor for QgsLayoutItemGroup, belonging to the specified ``layout``.
 %End
+    ~QgsLayoutItemGroup();
 
     virtual void cleanup();
 
diff --git a/src/core/layout/qgslayout.cpp b/src/core/layout/qgslayout.cpp
index 9baba1a8bf03..276e903c22d6 100644
--- a/src/core/layout/qgslayout.cpp
+++ b/src/core/layout/qgslayout.cpp
@@ -318,7 +318,7 @@ QgsLayoutItem *QgsLayout::layoutItemAt( QPointF position, const QgsLayoutItem *b
   }
 
   bool foundBelowItem = false;
-  for ( QGraphicsItem *graphicsItem : itemList )
+  for ( QGraphicsItem *graphicsItem : std::as_const( itemList ) )
   {
     QgsLayoutItem *layoutItem = dynamic_cast<QgsLayoutItem *>( graphicsItem );
     QgsLayoutItemPage *paperItem = dynamic_cast<QgsLayoutItemPage *>( layoutItem );
diff --git a/src/core/layout/qgslayoutframe.cpp b/src/core/layout/qgslayoutframe.cpp
index c6c513b7ecc4..e7161dc12a76 100644
--- a/src/core/layout/qgslayoutframe.cpp
+++ b/src/core/layout/qgslayoutframe.cpp
@@ -41,6 +41,11 @@ QgsLayoutFrame::QgsLayoutFrame( QgsLayout *layout, QgsLayoutMultiFrame *multiFra
   }
 }
 
+QgsLayoutFrame::~QgsLayoutFrame()
+{
+  QgsLayoutFrame::cleanup();
+}
+
 QgsLayoutFrame *QgsLayoutFrame::create( QgsLayout *layout )
 {
   return new QgsLayoutFrame( layout, nullptr );
@@ -158,6 +163,7 @@ void QgsLayoutFrame::cleanup()
 {
   if ( mMultiFrame )
     mMultiFrame->handleFrameRemoval( this );
+  mMultiFrame = nullptr;
 
   QgsLayoutItem::cleanup();
 }
diff --git a/src/core/layout/qgslayoutframe.h b/src/core/layout/qgslayoutframe.h
index 438da39580f9..ac2e135a8028 100644
--- a/src/core/layout/qgslayoutframe.h
+++ b/src/core/layout/qgslayoutframe.h
@@ -39,6 +39,7 @@ class CORE_EXPORT QgsLayoutFrame: public QgsLayoutItem
      * and belonging to a \a multiFrame.
      */
     QgsLayoutFrame( QgsLayout *layout, QgsLayoutMultiFrame *multiFrame );
+    ~QgsLayoutFrame() override;
 
     /**
      * Creates a new QgsLayoutFrame belonging to the specified \a layout.
diff --git a/src/core/layout/qgslayoutitem.cpp b/src/core/layout/qgslayoutitem.cpp
index 193093b930c6..8c3fc7e03499 100644
--- a/src/core/layout/qgslayoutitem.cpp
+++ b/src/core/layout/qgslayoutitem.cpp
@@ -95,7 +95,7 @@ QgsLayoutItem::QgsLayoutItem( QgsLayout *layout, bool manageZValue )
 
 QgsLayoutItem::~QgsLayoutItem()
 {
-  cleanup();
+  QgsLayoutItem::cleanup();
 }
 
 void QgsLayoutItem::cleanup()
diff --git a/src/core/layout/qgslayoutitemgroup.cpp b/src/core/layout/qgslayoutitemgroup.cpp
index 339c5f19c74f..48041cd30f7a 100644
--- a/src/core/layout/qgslayoutitemgroup.cpp
+++ b/src/core/layout/qgslayoutitemgroup.cpp
@@ -25,6 +25,11 @@ QgsLayoutItemGroup::QgsLayoutItemGroup( QgsLayout *layout )
   : QgsLayoutItem( layout )
 {}
 
+QgsLayoutItemGroup::~QgsLayoutItemGroup()
+{
+  QgsLayoutItemGroup::cleanup();
+}
+
 void QgsLayoutItemGroup::cleanup()
 {
   //loop through group members and remove them from the scene
diff --git a/src/core/layout/qgslayoutitemgroup.h b/src/core/layout/qgslayoutitemgroup.h
index fa9438584377..783a2f040a4c 100644
--- a/src/core/layout/qgslayoutitemgroup.h
+++ b/src/core/layout/qgslayoutitemgroup.h
@@ -35,6 +35,7 @@ class CORE_EXPORT QgsLayoutItemGroup: public QgsLayoutItem
      * Constructor for QgsLayoutItemGroup, belonging to the specified \a layout.
      */
     explicit QgsLayoutItemGroup( QgsLayout *layout );
+    ~QgsLayoutItemGroup() override;
 
     void cleanup() override;