Skip to content

Commit

Permalink
QgsAppMapTools: fix double-free during destruction of tools
Browse files Browse the repository at this point in the history
and do it also in QgsMapToolAddEllipse, QgsMapToolAddRectangle,
QgsMapToolAddRegularPolygon and QgsMapToolAddCircularString that
use a similar pattern

spotted by Valgrind when quitting the app:
```
==80600== Invalid read of size 8
==80600==    at 0x61C0F54: swap<QgsMapToolCaptureRubberBand*> (move.h:193)
==80600==    by 0x61C0F54: reset (unique_ptr.h:400)
==80600==    by 0x61C0F54: QgsMapToolCapture::deleteTempRubberBand() (qgsmaptoolcapture.cpp:848)
==80600==    by 0x4DF1017: QgsMapToolAddCircle::clean() (qgsmaptooladdcircle.cpp:135)
==80600==    by 0x4DF1390: QgsMapToolAddCircle::~QgsMapToolAddCircle() (qgsmaptooladdcircle.cpp:41)
==80600==    by 0x4DF36BC: QgsMapToolCircle2TangentsPoint::~QgsMapToolCircle2TangentsPoint() (qgsmaptoolcircle2tangentspoint.cpp:44)
==80600==    by 0x4E9516E: QgsAppMapTools::~QgsAppMapTools() (qgsappmaptools.cpp:185)
==80600==    by 0x4B327D2: operator() (unique_ptr.h:81)
==80600==    by 0x4B327D2: operator() (unique_ptr.h:75)
==80600==    by 0x4B327D2: reset (unique_ptr.h:402)
==80600==    by 0x4B327D2: QgisApp::~QgisApp() (qgisapp.cpp:1725)
==80600==    by 0x4B32E2C: QgisApp::~QgisApp() (qgisapp.cpp:1787)
==80600==    by 0x1163F7: main (main.cpp:1645)
==80600==  Address 0x2b25e608 is 200 bytes inside a block of size 616 free'd
==80600==    at 0x483D1CF: operator delete(void*, unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==80600==    by 0x4E9516E: QgsAppMapTools::~QgsAppMapTools() (qgsappmaptools.cpp:185)
==80600==    by 0x4B327D2: operator() (unique_ptr.h:81)
==80600==    by 0x4B327D2: operator() (unique_ptr.h:75)
==80600==    by 0x4B327D2: reset (unique_ptr.h:402)
==80600==    by 0x4B327D2: QgisApp::~QgisApp() (qgisapp.cpp:1725)
==80600==    by 0x4B32E2C: QgisApp::~QgisApp() (qgisapp.cpp:1787)
==80600==    by 0x1163F7: main (main.cpp:1645)
==80600==  Block was alloc'd at
==80600==    at 0x483BE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==80600==    by 0x4E956E2: QgsAppMapTools::QgsAppMapTools(QgsMapCanvas*, QgsAdvancedDigitizingDockWidget*) (qgsappmaptools.cpp:128)
==80600==    by 0x4B6409B: make_unique<QgsAppMapTools, QgsMapCanvas*&, QgsAdvancedDigitizingDockWidget*&> (unique_ptr.h:857)
==80600==    by 0x4B6409B: QgisApp::QgisApp(QSplashScreen*, bool, bool, QString const&, QString const&, QWidget*, QFlags<Qt::WindowType>) (qgisapp.cpp:1052)
==80600==    by 0x1157A7: main (main.cpp:1377)
```

The reason was that the mParentTool of QgsMapToolCapture was already
destroyed
  • Loading branch information
rouault authored and nyalldawson committed May 9, 2021
1 parent e8630d7 commit c195f46
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 13 deletions.
6 changes: 3 additions & 3 deletions src/app/qgsmaptooladdcircle.h
Expand Up @@ -51,9 +51,9 @@ class APP_EXPORT QgsMapToolAddCircle: public QgsMapToolCapture

/**
* The parent map tool, e.g. the add feature tool.
* Completed circle will be added to this tool by calling its addCurve() method.
**/
QgsMapToolCapture *mParentTool = nullptr;
* Completed circle will be added to this tool by calling its addCurve() method.
*/
QPointer<QgsMapToolCapture> mParentTool;
//! Circle points (in map coordinates)
QgsPointSequence mPoints;
//! The rubberband to show the circular string currently working on
Expand Down
2 changes: 1 addition & 1 deletion src/app/qgsmaptooladdcircularstring.h
Expand Up @@ -51,7 +51,7 @@ class APP_EXPORT QgsMapToolAddCircularString: public QgsMapToolCapture
* The parent map tool, e.g. the add feature tool.
* Completed circular strings will be added to this tool by calling its addCurve() method.
*/
QgsMapToolCapture *mParentTool = nullptr;
QPointer<QgsMapToolCapture> mParentTool;
//! Circular string points (in map coordinates)
QgsPointSequence mPoints;
//! The rubberband to show the already completed circular strings
Expand Down
6 changes: 3 additions & 3 deletions src/app/qgsmaptooladdellipse.h
Expand Up @@ -47,9 +47,9 @@ class APP_EXPORT QgsMapToolAddEllipse: public QgsMapToolCapture

/**
* The parent map tool, e.g. the add feature tool.
* Completed ellipse will be added to this tool by calling its toLineString() method.
**/
QgsMapToolCapture *mParentTool = nullptr;
* Completed ellipse will be added to this tool by calling its toLineString() method.
*/
QPointer<QgsMapToolCapture> mParentTool;
//! Ellipse points (in map coordinates)
QgsPointSequence mPoints;
//! The rubberband to show the ellipse currently working on
Expand Down
6 changes: 3 additions & 3 deletions src/app/qgsmaptooladdrectangle.h
Expand Up @@ -48,9 +48,9 @@ class APP_EXPORT QgsMapToolAddRectangle: public QgsMapToolCapture

/**
* The parent map tool, e.g. the add feature tool.
* Completed regular shape will be added to this tool by calling its addCurve() method.
**/
QgsMapToolCapture *mParentTool = nullptr;
* Completed regular shape will be added to this tool by calling its addCurve() method.
*/
QPointer<QgsMapToolCapture> mParentTool;
//! Regular Shape points (in map coordinates)
QgsPointSequence mPoints;
//! The rubberband to show the rectangle currently working on
Expand Down
6 changes: 3 additions & 3 deletions src/app/qgsmaptooladdregularpolygon.h
Expand Up @@ -56,9 +56,9 @@ class APP_EXPORT QgsMapToolAddRegularPolygon: public QgsMapToolCapture

/**
* The parent map tool, e.g. the add feature tool.
* Completed regular polygon will be added to this tool by calling its addCurve() method.
**/
QgsMapToolCapture *mParentTool = nullptr;
* Completed regular polygon will be added to this tool by calling its addCurve() method.
*/
QPointer<QgsMapToolCapture> mParentTool;
//! Regular Shape points (in map coordinates)
QgsPointSequence mPoints;
//! The rubberband to show the regular polygon currently working on
Expand Down

0 comments on commit c195f46

Please sign in to comment.