QDialog on stack+exec and dbus quit crash is no more

[David Faure]

On Thursday 12 December 2013 20:01:42 Albert Astals Cid wrote:
> El Dijous, 12 de desembre de 2013, a les 19:57:25, David Faure va escriure:
> > On Monday 11 November 2013 21:04:35 Jan Kundrát wrote:
> > > On Monday, 11 November 2013 19:17:22 CEST, Albert Astals Cid wrote:
> > > > Not sure you're understanding what i say, we have an explicit check
> > > > about
> > > > QDialog on stack+exec that says "it will crash if you dbus quit".
> > > 
> > > We've chatted about this with Albert on IRC. My understanding of this is
> > > that there are many situations which can trigger destruction of the
> > > parent
> > > object; one of them involves triggering the QAction "game_quit" via
> > > DBUS.
> > > As such, I consider the EBN check a valid warning, similar to compiler
> > > warning about other dubious code constructs like no parentheses around
> > > &&
> > > and ||, or unused variables -- might be valid in some circumstances, but
> > > should raise an eyebrow during review, and it might make sense to strive
> > > to
> > > make the code warning-free.
> > > 
> > > However, there's another bug, probably in the kdeui code. When I trigger
> > > that QAction, this is what valgrind reports:
> > > 
> > > ==355524== Invalid write of size 1
> > > ==355524==    at 0x5E3A6D7:
> > > QMenuPrivate::activateCausedStack(QList<QPointer<QWidget> > const&,
> > > QAction*, QAction::ActionEvent, bool) (qobject_p.h:321)
> > > ==355524==    by 0x5E412F2: QMenuPrivate::activateAction(QAction*,
> > > QAction::ActionEvent, bool) (qmenu.cpp:1130)
> > > ==355524==    by 0x5424A1A: KMenu::mouseReleaseEvent(QMouseEvent*)
> > > (kmenu.cpp:464)
> > > ==355524==    by 0x5A000DB: QWidget::event(QEvent*) (qwidget.cpp:8376)
> > > ==355524==    by 0x5E429DA: QMenu::event(QEvent*) (qmenu.cpp:2481)
> > > ==355524==    by 0x59A03BB: QApplicationPrivate::notify_helper(QObject*,
> > > QEvent*) (qapplication.cpp:4562)
> > > ==355524==    by 0x59A6168: QApplication::notify(QObject*, QEvent*)
> > > (qapplication.cpp:4105)
> > > ==355524==    by 0x533F265: KApplication::notify(QObject*, QEvent*)
> > > (kapplication.cpp:311)
> > > ==355524==    by 0x712AA3B: QCoreApplication::notifyInternal(QObject*,
> > > QEvent*) (qcoreapplication.cpp:949)
> > > ==355524==    by 0x59A1461:
> > > QApplicationPrivate::sendMouseEvent(QWidget*,
> > > QMouseEvent*, QWidget*, QWidget*, QWidget**, QPointer<QWidget>&, bool)
> > > (qcoreapplication.h:231)
> > > ==355524==    by 0x5A31BA0: QETWidget::translateMouseEvent(_XEvent
> > > const*)
> > > (qapplication_x11.cpp:4451)
> > > ==355524==    by 0x5A303B9: QApplication::x11ProcessEvent(_XEvent*)
> > > (qapplication_x11.cpp:3640)
> > > ==355524==  Address 0x165b1d21 is 529 bytes inside a block of size 728
> > > free'd
> > > ==355524==    at 0x4C2AE0C: operator delete(void*)
> > > (vg_replace_malloc.c:480)
> > > ==355524==    by 0x714715F: QObject::~QObject() (qscopedpointer.h:62)
> > > ==355524==    by 0x59FA0C9: QWidget::~QWidget() (qwidget.cpp:1554)
> > > ==355524==    by 0x54248CC: KMenu::~KMenu() (kmenu.cpp:168)
> > > ==355524==    by 0x7140783: QObjectPrivate::deleteChildren()
> > > (qobject.cpp:1907)
> > > ==355524==    by 0x59FA02C: QWidget::~QWidget() (qwidget.cpp:1679)
> > > ==355524==    by 0x5421315: KMainWindow::~KMainWindow()
> > > (kmainwindow.cpp:467)
> > > ==355524==    by 0x11A5AE: KMinesMainWindow::~KMinesMainWindow() (in
> > > /home/jkt/work/prog/kde/kmines/kmines)
> > > ==355524==    by 0x7147AC7: QObject::event(QEvent*) (qobject.cpp:1175)
> > > ==355524==    by 0x59FFDFA: QWidget::event(QEvent*) (qwidget.cpp:8846)
> > > ==355524==    by 0x5E13C8A: QMainWindow::event(QEvent*)
> > > (qmainwindow.cpp:1478)
> > > ==355524==    by 0x546E0C7: KXmlGuiWindow::event(QEvent*)
> > > (kxmlguiwindow.cpp:126)
> > > 
> > > I have no idea why the DBUS calls to QAction are handled as mouse
> > > events,
> > > but I suspect something fishy is going on here.
> > 
> > No, you're deleting the mainwindow while having the mouse over the menu,
> > so
> > when Qt finishes the mouse event handling it sends a mouse event to a
> > deleted widget, probably because all this happens within a nested event
> > loop, so Qt's main event loop didn't expect the widget to be already
> > deleted at that point.
> > 
> > (I'm confused about the testcase though - this is "calling dbus quit while
> > having the mouse over a menu?")
> 
> There was no mouse over the menu at all involved. Just dbus invoking the
> action.

Ah! Now I remember (yes, 7 months later).
This crash matches one of the items in my oversized todo list:

"Qt bug: QMenu crashes when deleted from one of its actions. kde bug 249884, 
RecursionGuard member accessed after deletion, weakpointer guard needed..."

Millian must have worked around the issue in kdevelop (the BR isn't really 
clear about the fix). I guess using deleteLater().

-- 
David Faure, faure at kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5