Developers: Qt Signal-Safety

After many years of writing Qt code, most of which has been non-GUI-based, I've gained a great deal of experience with signals and slots. One tricky issue about signals is that they are generally emitted when the QObject is not in a safe state to be deleted. This can often bite the user of such an object unexpectedly, especially when performing resets or invoking QMessageBox. The simple solution is to write all QObject classes in such a way that they are deletable as the result of any signal they emit. I've written an article on Signal-safety, where I outline the problem and solution, as well as provide workarounds for safely handling misbehaving objects (those that don't follow such guidelines). Every Qt developer should read and understand this. Happy coding!

Dot Categories: 

Comments

by Bojan (not verified)

Very good article. One remark though, I think that this line:
#define SAFESIGNAL(a) { QGuardedPtr self = this; a; if(!self) return; }
should be
#define SAFESIGNAL(a) { QGuardedPtr self = this; emit a; if(!self) return; }

by Allan Sandfeld (not verified)

I simply do not understand how QGuardedPtr will help in this case. QGuardedPtr will be set to zero when deleted, but if the owner does not use QGuardedPtr, it will still delete the object without setting the QGuardedPtr to zero.

I think most of this article is wrong, and should be reduced to the starting comment: DO NOT DELETE OBJECTS IN FUNCTIONS CALLED FROM THE DELETED OBJECT.

Be creative do something else, anything else..

by Philippe Fremy (not verified)

<< QGuardedPtr will be set to zero when deleted, but if the owner does not use QGuardedPtr, it will still delete the object without setting the QGuardedPtr to zero >>

No, when the QObject inherited class is deleted, it notifies all the QGuardedPtr and they all get set to 0.

by Frerich Raabe (not verified)

I think this macro is very dangerous and should be avoided, since it does more than just emitting a signal (as the name suggests - and it's not even good at that): it
a) assumes that the function has the return type 'void'
b) fails to do any cleanup work (freeing resources which have previously allocated by the function) if necessary.

In general, it's interesting to discuss about this problem but I think it's foolish to rely on guarantees like this, since it means you suddenly have to keep in mind which classes promise it and which don't. The Qt library does not give that promise (and as the article correctly states, explicitely warns about it) and I think you should not do so either. By assuming that deleting sender() in a slot is unsafe, you're always on the safe side.

by André Somers (not verified)

I agree with the above posters. It's not a good idea to delete an object from a function called by that object. It goes against the whole component idea lying behind the signal/slot paradigm.
If you do want to delete an object from a slot, I think I would give the signal an additional by reference boolean argument 'delete'. Set it to true in your slot, and let the object take care of deleting itself when it's ready to do so. That way, you're sure that no other objects are still to be called (a signal can be connected to more than one slot!) Of course, you still need to make sure you're not using any pointers to your deleted object anymore, but that is a matter of good software design, I'd say.

So: stick to the warning in the docs not to delete an object from a slot triggered by that object.

by Justin Karneges (not verified)

"It's not a good idea to delete an object from a function called by that object. It goes against the whole component idea lying behind the signal/slot paradigm."

Does it really go against anything? If you can't delete the sender in the slot, then you have to delay it, and delaying the delete is tricky business. Even QTimer is not necessarily enough (if sub-eventloops are invoked), which means that there is no guaranteed safe time to delete anything. You either need a safe-owner policy or a safe-child policy, and I've found that the latter is easier.

"a signal can be connected to more than one slot!"

Indeed, though I don't think it breaks any rules if one of the listeners misses out on receiving a signal.

by pahli_bar (not verified)

i would have to agree with previous poster about not deleting an object from a function called by that function. what are qt signal/slots - just wrappers around plain functions. its not a fire and forget kind of scenario - the function will get called and then only the processing will continue in the caller function. its something like this:
MyClass::mainFunc() {
....
dontDeleteMe(); // equivalent to signal emitted and function called

.... // any processing beyond this is not safe if SomeClass doesn't
//heed the warning of not deleting this class object

}

// the slot function - but treat as a part of the chain from the previous
// function
SomeClass::dontDeleteMe() {
delete MyClassObject;
}

so rule of thumb - DON'T delete caller class objects in the slot. One can always use a timer singleshot (as you indicated) to delete the caller if one really wants to do so.

SomeClass::dontDeleteMe() {
QTimer::singleShot(0, this, SLOT(deleteMyClass()));
}

SomeClass::deleteMyClass() {
delete MyClassObject;
}

here MyClassObject will be deleted only after all the signal processing is complete and the application returns to it event processing.

additionally, i don't agree about the part of not using a qdialog::exec(). very frequently one needs to do a yes/no type of query and do further processing based on the result of this query. without using an exec, one would to override the functions in the dialog class to find out the results.

pahli_bar

by Justin Karneges (not verified)

You really should read the article and my comments here. QTimer doesn't solve everything. Your example above is essentially the same as deleteLater(), which is error-prone if you mix it with QDialog::exec(). I cover this case in my article. Of course, your example would work for your specific purpose, but my goal here was to find a generic solution. Instead of having to guess or deal with slots safely depending on what is going on, I wanted to come up with something that would work everywhere, all the time.

The fully generic user-side solution is a lot of code, certainly not something you'd want to write for every slot, and this is why I made the 'SafeDelete' class. You don't need to prove to me that it is possible to secure things on the user-side, as I've already demonstrated this with SafeDelete.

However, you have to admit that solving the problem in the object is a heck of a lot simpler and saner. Best of all, writing a signal-safe class does not require the user to change anything. He simply benefits from increased crash-resistance. What is there to lose?

by Justin Karneges (not verified)

Personally I don't use the macro in my own code, it was just a way to illustrate how straightforward the concept is, as opposed to the QTimer method which requires a lot more thought and planning. I definitely agree with your points (a) and (b), and you'll notice that in the 'legacy' section I don't use the macro, as I need to free resources before returning.

I have encountered so many instances where programs explode in cleanup cases, due to a signal being on the call stack somewhere. If we rule out the use of delete, deleteLater(), and QTimer as the response of a signal, then cleanup is impossible. If you're careful about invoking sub-eventloops, then you can at least use the latter two.

In my experience, I've found that it is easier for the child object to behave itself than for the owner to make workarounds.

by Jelmer Feenstra (not verified)

I think it's good to note he's also the (primary) author of the wonderful Jabber client called Psi (http://psi.affinix.com).

Check it out !

by Nick (not verified)

After reading this article I thought: Isn't it a pitty that Qt doesn't use exceptions?

by Some KDE user (not verified)

I agree, using exceptions would be technically superior.

However, given the range of platforms Trolltech wants to support with QT, that they do not require exception support is probably a good (business) decision for them.

But a more grave problem I think is, that building QT with exception support is not encouraged by Trolltech and KDE. This way integration of code which uses exceptions is not naturally possible.

by J6t (not verified)

I agree entirely. Exceptions would make things much, much easier to code. In one of my projects I've converted a return-code based function call hierarchy to exceptions, and saved about 1/4th of the lines of code (and many indentation levels).

IMHO it was a bad design decision that Qt doesn't work with exceptions. It is one of the major reasons why I would not consider it for my next business class project.

-- Hannes

by Akito Nozaki (not verified)

wrote up more solution to the problem.

spent way too much time not to post :)
http://tanoshi.net/signalsafety-ext.html

by Peaker (not verified)

How about the following alternate solution:

Deleter.h:

class Deleter {
public:
void deleteRegistered();
void registerForDeletion(QObject *);
...
};

extern Deleter deleter;

And in main.cpp:

while(..) {
// Let the Main loop run for a few ticks
deleter->deleteRegistered();
}

And delete stuff via registerForDeletion(..);

This guarantees what you wanted: That there's no signal on the call stack when the actual deletion takes place.