Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #30028 Closing Vertex Edititor crashes the app #30030

Merged
merged 1 commit into from May 31, 2019

Conversation

PeterPetrik
Copy link
Contributor

fix #30028

double delete of mVertexEditor (unique_ptr in context of Qt's events)

@PeterPetrik PeterPetrik added backport release-3_4 Bug Either a bug report, or a bug fix. Let's hope for the latter! labels May 30, 2019
Copy link
Member

@wonder-sk wonder-sk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. I think we should not use unique_ptr for QObject-based classes in QGIS code to avoid exactly this kind of crashes due to forced delete too early.

@@ -313,6 +313,8 @@ QgsVertexTool::~QgsVertexTool()
delete mVertexBand;
delete mEdgeBand;
delete mEndpointMarker;
if ( mVertexEditor )
delete mVertexEditor;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to do "if (x) delete x;" - just do "delete x;"
and we probably don't need to do that anyway since the vertex editor widget has a parent (which will take care of its deletion)

// do not delete immediately because vertex editor
// can be still used in the qt event loop
mVertexEditor->deleteLater();
mVertexEditor = nullptr;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not necessary to do mVertexEditor = nullptr; if we are using QPointer (it would be needed if we used raw pointer)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Closing Vertex Edititor crashes the app.
2 participants