Avoid calling overridden virtual method in base class destructor

Fixes some undefined behavior when deleting layout items
qgis · Oct 3, 2023 · 17a34a1 · 17a34a1
1 parent a3a0e7b
commit 17a34a1
Show file tree

Hide file tree

Showing 8 changed files with 17 additions and 2 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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;