Want to see how to crash almost every KDE application?

• In your favorite KDE application, open any modal dialog, e.g. the configuration dialog.
• Quit the application using D-BUS, like this:


qdbus org.kde.kontact /MainApplication quit


Chances are high that your application just bit the dust and you're greeted by Dr. Konqui.

Why does it happen? Well, let's have a look at how modal dialogs are usually created. Most often applications have some "main widget" with slots reacting to user actions, creating a modal dialog on the stack, like this:


void ParentWidget::slotDoSomething() {
SomeDialog dlg( this ); //the dlg-on-stack-variant
if ( dlg.exec() == QDialog::Accepted ) {
const QString str = dlg.someUserInput();
//do something with with str
}
}


What could possibly go wrong here? Nothing, you might say. Looks safe, right? This code is so trivial and the pattern is used in so many Qt and KDE applications. Wrong! This code is bound to cause a crash in a couple of situations, where a D-BUS call to quit() is only one of them (and a somewhat esoteric one, admittedly). Why?

While dlg.exec() is executed and the dialog is shown, your application returns to the event loop and processes any event that comes along. Which means, that anything might happen before exec() returns. Right. Anything. In the usual case, the user enters something and then presses either OK or Cancel to close the dialog, and exec() returns. He can't to anything else, as the parent widget is disabled anyway, as the dialog is modal. But in most non-trivial applications, there are more things going on: Asynchronous background jobs might complete, e.g. Konqueror might finish loading a website and replace the currently displayed Kate KPart while you're in its print dialog configuring your printout. KMail might start an interval mail check while a (modal) KMessageBox still tells you that it couldn't reach your mail server last time. Or something is triggered by a D-BUS call.

That's fine most of the time, unless in the following case: The background operation/d-bus call triggers the deletion of the dialog's parent, the ParentWidget instance which created SomeDialog dlg in its slotDoSomething(). When the ParentWidget is deleted, dlg is deleted in turn, as it's the child of ParentWidget. Thus when exec() returns, we are in an awkward situation: The execution of the parent widget's slotDoSomething is continued, although "this" is already deleted, and so is dlg, which was created on the stack (!). Access to a member variable of the parent widget will crash the application, and if not, it will crash when leaving slotDoSomething, as it tries to destroy the dlg object a second time when unwinding the stack. Booom.

This happens more often than you think, in particular it's a problem in KParts which can be deleted at any time by their host application, like Konqueror does in the example above. If your code is part of a library, you can even bet that it will be used in such a scenario. So better fix it.

But how? Well, you can avoid the crash by not passing "this" as parent. That's a bad idea though, as not passing a parent to a modal dialog breaks window stacking and has all kinds of bad effects. That's particularly bad on OS X, where the dialog might pop up behind (!) the blocked (!) parent widget, unreachable for the user. Which renders the application useless and isn't much better than a crash, from a user's perspective. Effects on other window systems might be less worse, but annoying nevertheless (like modal dialogs creating other modal dialogs without passing themselves as parent).

Ok, deleting an object created on the stack might be a bad idea. Let's create it on the heap then:


void ParentWidget::slotDoSomething() {
SomeDialog* dlg = new SomeDialog( this ); //the dlg-on-heap-variant
if ( dlg->exec() == QDialog::Accepted ) {
const QString str = dlg->someUserInput();
//do something with with str
}
delete dlg;
}


Now you have to delete the dialog to not leak memory. But if the dialog was already deleted before the return of exec()? Then you delete a now invalid pointer to a already deleted object. Booom, again.

So, is there a solution? Yes, use QPointer:


void ParentWidget::slotDoSomething() {
QPointer dlg = new SomeDialog( this ); //the dlg-on-heap-variant
if ( dlg->exec() == QDialog::Accepted ) {
const QString str = dlg->someUserInput();
//do something with with str
}
delete dlg;
}


If the dialog is deleted, the QPointer is reset to 0, thus dlg is either still valid or was reset to 0 when exec() returns, and "delete dlg" is safe as deleting null pointers does no harm. If was deleted during the exec(), exec() will return QDialog::Rejected. If there is more to be done in the slot after the exec(), especially in the Rejected-Case, you should check for dlg != 0, to find out if dlg and "this" were deleted, to avoid further method calls or member access that would dereference "this".

Conclusion: Use a QPointer when showing modal dialogs via exec(). It avoids crashes caused by deleting the dialog's parent widget, which can be triggered by an event (background job, D-BUS call, ...) before dlg->exec() returns. In general, all event processing, using exec() or otherwise, is unsafe unless you take special measures like done here. This situation occurs more often than one might think at first glance, especially if your parent widget might be created inside a KPart, which can be created and destroyed any time by their host application (e.g., Konqueror).

Update: As Eean points out in the comments, exec() is only one of its kind of methods returning to the event loop behind the scenes. Other synchronous convenience APIs around asynchronous operations like e.g. KJob::exec() or QProcess::waitForStarted/Finished() have the same problem. QDialog::exec() is just the one used all the time. Ideally, such methods would be avoided alltogether. He's right, and the question how to design an application to avoid such issues is worth another article.

And, btw, if you absolutely need to call a method which will process events, there is another cheap trick to check if you're still alive. You can use a QPointer to yourself (works only from within QObjects, obviously) like this:


QPointer that( this );
...call the method which processes events
if ( !that )
return;
...continue


It has the advantage that also works with the static KMessageBox::foo() functions.