Skip to content

Commit

Permalink
When deleting a map canvas, first clear all the pointers to that
Browse files Browse the repository at this point in the history
canvas from child map tools

Otherwise we risk the map tools trying to do things with the
canvas within their cleanup code, and at that stage the canvas
is already partially destroyed.

On qt 5 builds this leads to undefined behavior, and on qt 6
builds it triggers a newly introduced assert designed to
catch these kinds of bugs.

It's possible that we should just delete the map tool children
upfront here instead, but that's a little more risky.
  • Loading branch information
nyalldawson committed Aug 19, 2022
1 parent f571d0a commit 503783b
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 0 deletions.
12 changes: 12 additions & 0 deletions src/gui/qgsmapcanvas.cpp
Expand Up @@ -253,8 +253,20 @@ QgsMapCanvas::~QgsMapCanvas()
if ( mMapTool )
{
mMapTool->deactivate();
disconnect( mMapTool, &QObject::destroyed, this, &QgsMapCanvas::mapToolDestroyed );
mMapTool = nullptr;
}

// we also clear the canvas pointer for all child map tools. We're now in a partially destroyed state and it's
// no longer safe for map tools to try to cleanup things in the canvas during their destruction (such as removing
// associated canvas items)
// NOTE -- it may be better to just delete the map tool children here upfront?
const QList< QgsMapTool * > tools = findChildren< QgsMapTool *>();
for ( QgsMapTool *tool : tools )
{
tool->mCanvas = nullptr;
}

mLastNonZoomMapTool = nullptr;

cancelJobs();
Expand Down
1 change: 1 addition & 0 deletions src/gui/qgsmaptool.h
Expand Up @@ -353,6 +353,7 @@ class GUI_EXPORT QgsMapTool : public QObject
//! The translated name of the map tool
QString mToolName;

friend class QgsMapCanvas;
friend class TestQgsMapToolEdit;

};
Expand Down

0 comments on commit 503783b

Please sign in to comment.