How to cancel pending signals?

[dictoon]

I have a QTreeWidget. When the user selects an item (of type QTreeWidgetItem) and presses the Delete key, a slot is invoked on this item to delete itself.

Under certain circumstances, deleting an item might take long enough for the user to press Delete a second time on the same item, leading in my case to a crash.

What is the best way to handle this situation?

I'm using Qt 4.8.6.

[anda_skoa]

You could take or hide the item instead of deleting it, and then call a deletion function using QMetaObject::invokeMethod() with Qt::QueuedConnection.

Cheers,
_

[dictoon]

Thanks for your answer anda_skoa, but I'm not sure I fully understand.

Right now, I create a context menu (QMenu) with the QTreeWidget as parent, then add an action to that menu that will delete the selected item(s):



QMenu* menu = new QMenu(treeWidget());
...
menu->addAction("Delete", this, SLOT(slot_delete()), QKeySequence(Qt::Key_Delete));
...
menu->exec(treeWidget()->mapToGlobal(point));
And here is a slot_delete():



void MyItem::slot_delete()
{
// Do a bunch of stuff here, might take some time.

delete this;
}
How would I change this?

I'm not sure how first hiding or taking out the item in slot_delete() would fix the problem.

[d_stranz]

How would I change this?

If slot_delete() really *is* a slot, then it is impossible for the user to press Delete a second time and have the slot interrupted. All of the processing in a slot is performed and then the function exits and control returns to the event loop.

What is probably happening in your case is that you aren't clearing whatever data structure you use to keep track of the pointer to the "selected item", so that when the event loop gets around to processing the second Delete signal, the item you are showing as the "selected item" is already gone, and you try to access it through the now invalid pointer. That's what causes your crash.

So you need to truly clean up everything associated with the item, including places where a pointer to it might be stored *before* leaving your slot.

In my opinion, though, a user interface design that performs an irreversible action (like deleting something) without giving the user a way to say, "Oops, I didn't mean that!" is not one that will make people happy for very long. An undo stack, a confirmation dialog that asks "Are you sure?", whatever.

[dictoon]

Thanks for chiming in d_stranz.


If slot_delete() really *is* a slot, then it is impossible for the user to press Delete a second time and have the slot interrupted.

It's not like the slot execution gets interrupted; my impression is rather that Qt queues up further signals for this item, but the item no longer exists after the first slot_delete() invocation.

I think you're right though, our actual code is slightly different than what I pasted here, and that's likely the cause of our problem:

slot_delete(): https://github.com/appleseedhq/appleseed/blob/80fe6e3cc57ab90989f1428713f1cd7a097a6b26/src/appleseed.studio/mainwindow/project/objectinstanceitem.cpp#L474-L479
schedule_or_execute(): https://github.com/appleseedhq/appleseed/blob/80fe6e3cc57ab90989f1428713f1cd7a097a6b26/src/appleseed.studio/mainwindow/rendering/renderingmanager.cpp#L236-L247
EntityDeleteAction: https://github.com/appleseedhq/appleseed/blob/80fe6e3cc57ab90989f1428713f1cd7a097a6b26/src/appleseed.studio/mainwindow/project/entityactions.h#L123-L142
Actual deletion code: https://github.com/appleseedhq/appleseed/blob/80fe6e3cc57ab90989f1428713f1cd7a097a6b26/src/appleseed.studio/mainwindow/project/objectinstanceitem.cpp#L481-L502


EDIT: Now it's clear: we cannot delete items during rendering, so in that case we queue deletion actions. Of course all deletion actions after the first one will operate on an item that no longer exists. Somehow we need to check if the item still exists...


In my opinion, though, a user interface design that performs an irreversible action (like deleting something) without giving the user a way to say, "Oops, I didn't mean that!" is not one that will make people happy for very long.

Of course, but this seems like a completely orthogonal issue to what we are discussing here.

(We'll go for an undo stack.)

[d_stranz]

EDIT: Now it's clear: we cannot delete items during rendering, so in that case we queue deletion actions.

No, you still don't understand - you are deleting the item in your slot, but there are references to the pointer that are stored elsewhere that aren't getting cleaned up before leaving the slot. There is no rendering going on while your slot is executing - it can only happen *after* leaving the slot. There is no need to queue anything or take any other special action other than to ensure that *anything* that is storing a reference to the object you have just deleted is cleaned up prior to leaving the slot.

Note that this doesn't have to be something in *your* code - if the object you are deleting inherits from QObject, then there could be references you don't know about that Qt maintains. To avoid this, you probably *don't* want to call "delete this" in the slot. Instead, you call "deleteLater()", which will ensure that any references to the object through the QObject system are also cleaned up. The next time the event loop runs, the instance will be cleaned up after, and then will get deleted.

[dictoon]

No, you still don't understand - you are deleting the item in your slot

Yes, I do understand the problem now (thanks to your heads up in your first post), but maybe I didn't explain well what we're doing wrong:

This is the UI of a debug/inspection tool for an offline rendering engine. We cannot delete entities from the scene (*our* scene) while our program is rendering. I'm not talking about Qt rendering here, I'm talking about our program doing its computational business (in other threads). So, during rendering, instead of simply deleting an item when the user selects one and presses Delete, we *queue* the deletion action (using *our* queue mechanism). Obviously at this point the item still exists in the UI and is still available to the user, and nothing prevents him from deleting again the same item.

So, as you hinted, there *are* other references in our system (in the action queue, specifically) to the item that was just deleted. We need to implement a mechanism to either remove conflicting/non-longer-applying actions from the action queue, or to limit the size of the action queue to 1 action, or something else.

Incidentally, switching to deleteLater() might be a good option, even though 'delete this' never caused any problems.

Cheers,
Franz

[d_stranz]

OK, that's a little more clear. I found use of deleteLater() was required when I was removing items from a tree view (if I remember correctly). Simply calling delete would result in a crash, presumably due to dangling references.

I have used the QObject::destroyed() signal successfully as part of a cleanup mechanism. As the docs say, it is emitted immediately before the instance is deleted, so you can still use the pointer if needed.