[KDE/Mac] Review Request 120150: [OS X] prevent another crash after finishing a code difference review

[Milian Wolff]

> On Sept. 12, 2014, 5:31 p.m., René J.V. Bertin wrote:
> > So, if I activate malloc/free guarding too early (in PatchReviewPlugin::setPatch, when called with a "real" patch, not at plugin load time), I get this:
> > 
> > ### starting malloc guarding 
> > ### PatchReviewPlugin::setPatch() deleting previous patch LocalPatchSource(0x1237b3830) "Custom Patch" with file KUrl("") basedir KUrl("") 
> > setting new patch "VCS Diff" with file KUrl("file:///private/var/folders/W2/W27-dhO62RWW-U+BYnQ9i++++TY/-Tmp-/kde-bertin/kdevelopC890942.patch") basedir KUrl("file:///Volumes/Debian/Users/bertin/work/src/new/KDE/kdevelop/kdevplatform-git-1.7.60/") 
> > kdevelop.bin(89094,0x7fff70affcc0) malloc: *** error for object 0x138a80f10: Non-aligned pointer being freed (2)
> > *** set a breakpoint in malloc_error_break to debug
> > 
> > Program received signal SIGABRT, Aborted.
> > 0x00007fff85f470b6 in __kill ()
> > (gdb) bt
> > #0  0x00007fff85f470b6 in __kill ()
> > #1  0x00007fff85fe79f6 in abort ()
> > #2  0x00007fff85fd662d in szone_error ()
> > #3  0x00007fff827358d4 in -[NSViewHierarchyLock dealloc] ()
> > #4  0x00007fff8273443c in -[NSWindow dealloc] ()
> > #5  0x0000000101ed6be2 in -[QCocoaPanel dealloc] ()
> > #6  0x0000000101ed12e0 in QWidget::destroy ()
> > #7  0x0000000101f89222 in QWidget::~QWidget ()
> > #8  0x000000012926f985 in ProjectTreeView::popupContextMenu (this=<value temporarily unavailable, due to optimizations>, pos=<value temporarily unavailable, due to optimizations>) at /Volumes/Debian/MP6/var/macports/build/_Volumes_Debian_MP6_site-ports_kde_kdevplatform-git/kdevplatform-git/work/kdevplatform-git-1.7.60/plugins/projectmanagerview/projecttreeview.cpp:316
> > Previous frame inner to this frame (gdb could not unwind past this frame)
> > 
> > 
> > In other words, I expose a situation that either doesn't occur or is handled silently without malloc guarding, and it gets triggered because apparently the popup menu's breakdown happens concurrently with the patchreview toolview's initialisation.
> > 
> > Is there a place in patchreview.cpp where I can assume that the popup menu through which I get to the toolview is completely gone (and any malloc/free errors exposed are in the plugin code)?
> 
> Milian Wolff wrote:
>     This looks like there is an issue with deleting the context menu widget, probably due to nested eventloop madness and such.
>     
>     We might need to replace the stack-allocated QMenu's with heap-allocated ones and replace the blocking `QMenu::exec()` with the non-blocking `QMenu::popup()`. This removes the nested eventloop.
>     
>     If you feel like doing that cleanup, it would be greatly appreciated. Generally though, it might make more sense to do this in the frameworks branch as you'll need to change quite a bit of code, I guess.
>     
>     And to reiterate: None of the above is OS-X specific. You might just be "lucky" in seeing a reliable crash which just randomly happens to work on Linux, thus noone saw the issues.
> 
> René J.V. Bertin wrote:
>     madness is indeed the word, I keep having to force my mind around the implications of using that blocking call. Yes, the context menu is supposed to be released before we're really out of the function, but no, the patch review thingy's activation doesn't wait for that.
>     I'm not sure if I've already mentioned it on here, but the "frameworks branch" (as in the KF5 version) is still a while off on OS X, and my quick attempt at installing Project Neon5 to build kdevelop under KF5 on Linux didn't get very far. So much as I'd appreciate to do *that cleanup*, it'll have to wait a bit :)
>     
>     Unless there's an intermediate solution. Is it not possible to set the context menu to return the activated item, or the slot to trigger, so as to defer that at least until after the menu's event loop has exited? I know it's an oldfashioned and maybe even uncool approach, but this one ought to make debugging easier ;)

You can try to make the connection to the QAction a Qt::QueuedConnection. Maybe that helps.


- Milian


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


On Sept. 12, 2014, 8:30 p.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120150/
> -----------------------------------------------------------
> 
> (Updated Sept. 12, 2014, 8:30 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDevelop and Olivier Goffart.
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> In https://reviewboard.kde.org/r/120081/ I proposed an (accepted) approach to prevent kdevelop from crashing after closing the patch review ("git/show differences") toolview. I had another of those crashes after heavier-than-usual perusal of the toolview, despite the previous patch. The attached patch replaces all `delete`s of `QObject` derived class instances with `deleteLater()`.
> 
> 
> Diffs
> -----
> 
>   plugins/patchreview/patchreview.cpp 18b63db 
> 
> Diff: https://git.reviewboard.kde.org/r/120150/diff/
> 
> 
> Testing
> -------
> 
> kdevplatform git/kde4-legacy on KDE/MacPorts OS X 10.6.8 . 
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-mac/attachments/20140913/b9bc5464/attachment.html>