-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118858/
-----------------------------------------------------------

(Updated June 22, 2014, 7:58 a.m.)


Review request for kdelibs.


Changes
-------

Thanks for your comments, Christoph and Dominik! It's interesting that this 
kind of crash was already discussed in 2009. I'd like to emphasize though that 
the problem is not limited to the "quit via D-Bus" use case. None of the 
reporters of the KUrlNavigatorCrash mention D-Bus in their reports. Lots of 
other things can happen inside nested event loops: timers can fire, slots can 
be called via queued connections, etc. Some of these things might delete 
objects unexpectedly, and cause crashes which are very hard to reproduce.

Trying the "D-Bus quit" method for every dialog and context menu in every 
application might actually reveal more real bugs that cause rare random 
crashes.  

I did find another nested event loop-related bug in KUrlNavigator: in 
KUrlNavigator::Private::openContextMenu(): the menu, which is a child of the 
KUrlNavigator, is created on the stack. This causes a crash if the navigator is 
closed inside the loop because the QObject destructor tries to delete the child 
object on the stack.

The fix is quite straightforward: move the menu to the heap, and use the same 
"delete the menu if the QPointer is not null" idea like in 
KUrlNavigator::Private::openPathSelectorMenu().

If noone sees a problem with the second version of the patch, I'll commit it in 
the next few days and forward-port to KF5.


Bugs: 293863
    http://bugs.kde.org/show_bug.cgi?id=293863


Repository: kdelibs


Description
-------

KUrlNavigator opens menus with exec() in a few places, and accesses member 
variables or pointers to children after that. This can cause crashes if the 
object has been deleted inside the nested event loops.

This can be fixed by using QPointers to detect if an object was deleted 
already, and return immediately in that case.


Diffs (updated)
-----

  kfile/kurlnavigator.cpp f5dfc81 
  kfile/kurlnavigatorbutton.cpp 6cb40b1 

Diff: https://git.reviewboard.kde.org/r/118858/diff/


Testing
-------

Cannot reproduce the crashes any more. The menus in KUrlNavigator still work 
fine for me.


Thanks,

Frank Reininghaus

Reply via email to