Diffusion KDevPlatform 7cda7c95d5bf

Fix crashy dialogs (found by krazy and by hand)
7cda7c95d5bf
Actions

Authored by croick on Jun 30 2017, 9:36 PM.

Description

Fix crashy dialogs (found by krazy and by hand)

Summary:

use a wrapper class which simplifies the use of QPointer for the creation of local QDialogs
some false positives are marked
some dialogs not identified by krazy have the main window as parent, they may still cause a crash, when the program is killed

Test Plan:
Before:

open "About KDevelop Platform" dialog
"killall kdevelop" -> kdevelop closes (as intended), but with crash

After:

open same dialog
"killall kdevelop" -> kdevelop silently quits

Reviewers: KDevelop, apol

Reviewed By: KDevelop, apol

Subscribers: apol, anthonyfieroni, kdevelop-devel

Differential Revision: https://phabricator.kde.org/D6308

Details

Committed

croick

Jun 30 2017, 9:57 PM

Reviewer

KDevelop

Differential Revision

D6308: Fix crashy dialogs (found by krazy and by hand)

Parents

R33:c6dd6e648481: Merge branch '5.1'

Branches

Unknown

Tags

Unknown

croick committed R33:7cda7c95d5bf: Fix crashy dialogs (found by krazy and by hand) (authored by croick).Jun 30 2017, 9:57 PM

croick mentioned this in R33:27fc9e3396cc: Fix releasing semaphore (bug introduced by commit 7cda7c95d5bf).Jul 1 2017, 8:00 PM

Hmm. Why not use dialog->setAttribute(Qt::WA_DeleteOnClose) instead? That seems to be the standard way to do it, no?

In R33:7cda7c95d5bfa93f34c5a52df17480c248a8367c#2010755, @brauch wrote:

Hmm. Why not use dialog->setAttribute(Qt::WA_DeleteOnClose) instead? That seems to be the standard way to do it, no?

It would not be possible to access elements of the dialog after exec(), which is required more often than not.

In R33:7cda7c95d5bfa93f34c5a52df17480c248a8367c#2010776, @croick wrote:

In R33:7cda7c95d5bfa93f34c5a52df17480c248a8367c#2010755, @brauch wrote:

Hmm. Why not use dialog->setAttribute(Qt::WA_DeleteOnClose) instead? That seems to be the standard way to do it, no?

It would not be possible to access elements of the dialog after exec(), which is required more often than not.

I don't think that's true, it will only be deleted when the main event loop is next entered.

In R33:7cda7c95d5bfa93f34c5a52df17480c248a8367c#2010784, @brauch wrote:

In R33:7cda7c95d5bfa93f34c5a52df17480c248a8367c#2010776, @croick wrote:

In R33:7cda7c95d5bfa93f34c5a52df17480c248a8367c#2010755, @brauch wrote:

Hmm. Why not use dialog->setAttribute(Qt::WA_DeleteOnClose) instead? That seems to be the standard way to do it, no?

It would not be possible to access elements of the dialog after exec(), which is required more often than not.

I don't think that's true, it will only be deleted when the main event loop is next entered.

But

delete this;

is called before exec() returns. See https://github.com/qt/qtbase/blob/5.9/src/widgets/dialogs/qdialog.cpp#L553 for reference.

Oh, sorry then.

signature.asc849 BDownload

In R33:7cda7c95d5bfa93f34c5a52df17480c248a8367c#2010924, @brauch wrote:

Oh, sorry then.

signature.asc849 BDownload

No problem, it made me learn about this as well :)

Changes (22)

				Path	Packages
	M			language/codegen/basicrefactoring.cpp
	M			plugins/appwizard/appwizardplugin.cpp
	M			plugins/appwizard/projectselectionpage.cpp
	M			plugins/externalscript/externalscriptview.cpp
	M			plugins/filetemplates/templateclassassistant.cpp
	M			plugins/filetemplates/templateselectionpage.cpp
	M			plugins/git/gitplugin.cpp
	M			plugins/subversion/kdevsvnplugin.cpp
	M			plugins/subversion/svnjobbase.cpp
	M			shell/documentcontroller.cpp
	M			shell/environmentconfigurebutton.cpp
	M			shell/loadedpluginsdialog.cpp
	M			shell/mainwindow_actions.cpp
	M			shell/projectcontroller.cpp
	M			shell/sessioncontroller.cpp
	M			shell/settings/environmentwidget.cpp
	M			shell/settings/sourceformattersettings.cpp
	M			shell/settings/templatepage.cpp
	M			shell/uicontroller.cpp
	A			util/scopeddialog.h
	M			vcs/dvcs/dvcsplugin.cpp
	M			vcs/vcspluginhelper.cpp