Differential D1786

Fix crash with blocking Open dialog
ClosedPublic
Actions

Authored by elvisangelaccio on Jun 7 2016, 10:21 AM.

Details

Reviewers
rthomsen
Commits
R36:0d16e7d028a0: Fix crash with blocking Open dialog
Summary

QDialog::exec() is bad and should not be used unless strictly necessary. The exec() call is blocking and anything can happen before it returns to the caller. The old workaround (which Ark uses) is to use a QPointer to guard the dialog. Yet I can reproduce a crash by calling QCoreApplication::quit() via dbus while the dialog is open.

The proper fix is to call the dialog in a non-blocking way. Nowadays this is trivial thanks to C++11 lambdas.

(Patch meant for Applications/16.04)

Test Plan

Steps to reproduce the crash (without the patch):

  • Run ark and open the "Open Archive" dialog (CTRL+O)
  • Run qdbusviewer
  • Type ark in the search bar
  • Call the MainApplication/org.qtproject.Qt.QCoreApplication/quit method (right-click on it)

Diff Detail

Repository
R36 Ark
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
elvisangelaccio updated this revision to Diff 4266.Jun 7 2016, 10:21 AM
elvisangelaccio retitled this revision from to Fix crash with blocking Open dialog.
elvisangelaccio updated this object.
elvisangelaccio edited the test plan for this revision. (Show Details)
elvisangelaccio added a reviewer: rthomsen.
elvisangelaccio set the repository for this revision to R36 Ark.
elvisangelaccio added a project: Ark.
elvisangelaccio updated this object.
elvisangelaccio added a subscriber: kde-utils-devel.
elvisangelaccio updated this revision to Diff 4274.Jun 7 2016, 11:02 AM

Call deleteLater() also when the dialog is rejected.

rthomsen accepted this revision.Jun 9 2016, 8:24 AM
rthomsen edited edge metadata.

Tested and solves the crash :)

This revision is now accepted and ready to land.Jun 9 2016, 8:24 AM
Closed by commit R36:0d16e7d028a0: Fix crash with blocking Open dialog (authored by elvisangelaccio). · Explain WhyJun 9 2016, 9:25 AM
This revision was automatically updated to reflect the committed changes.
elvisangelaccio mentioned this in R36:0d16e7d028a0: Fix crash with blocking Open dialog.

Revision Contents
Changeset List

PathPackages
Mapp/mainwindow.cpp (16 lines)

Diff 4301

View Options

app/mainwindow.cpp

Loading...