Review Request 120420: [OS X] a more complete approach to prevent the crash after finishing a code-difference review or git/commit

[René J.V. Bertin]

> On Dec. 19, 2014, 6:59 p.m., René J.V. Bertin wrote:
> > I don't know whether it's related to my proposed changes or to something else, but I am now experiencing another issue.
> > 
> > After submitting a patch to ReviewBoard, *most of the time* KDevelop enters what I can only describe as an "in-between" state. The actions in the Mac's global menubar remain functional, but when I try to open a context menu by right-clicking on an entry in the project toolview, a kind of UI freeze occurs. Again, the global menu items remain functional, and allow me to quit KDevelop (which is about the only sensible thing to do at this point).
> > Attaching a debugger and going over backtraces obtained after trying too hard to get back into a working state it would seem that the right-click context menu *did and remains open* but simply never got rendered. (In fact, there's a brief flicker, and a visual indication in the project tree where I right-clicked.) Hitting escape is no help, though.
> > 
> > This only happens after submitting a patch for review successfully.
> > 
> > Any ideas?

It's possible too that this has something to do with the much newer OS X version I'm running now, 10.9.5 ...


- René J.V.


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


On Dec. 19, 2014, 6:52 p.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120420/
> -----------------------------------------------------------
> 
> (Updated Dec. 19, 2014, 6:52 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDevelop.
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> This RR replaces https://git.reviewboard.kde.org/r/120150/, itself a follow-up to RR https://reviewboard.kde.org/r/120081/ . In that RR I proposed an (accepted & submitted ) approach to prevent kdevelop from crashing after closing the patch review ("git/show differences") toolview.
> 
> This turned out to be insufficient, the most likely culprit being a nested event loop. In my most recent bug hunting I noticed that `closeReview` is called through Qt's mouse-eventh handling function when the "Finish Review" button (or "Commit", when doing a git->commit) is pressed. The crash I have been experiencing occurs in that function (or at least not far down the call stack from that function).
> I had already noticed that the crash never occurred when I closed all document views first (the patch file and all modified files), e.g. by using the "Close All" menu action.
> 
> So:
> - `closeReview` closes all open documents (minus the patchfile) via `qDeleteAll(m_highlighters)` or by calling `m_highlighters.erase(iterator)` repeatedly. Documents are thus closed by deleting the "content object" that represents them, letting that fact filter back to the UI
> - closing documents via the UI takes the opposite route, and is more in line with a modern GUI framework (at least OS X/Cocoa) where everything happens because of an event in the user interface. You receive an event (message) when the user closes a document, the framework handles most UI-related stuff for you, and if you have to dispose of document-related non-gui data you can do that for example just before the widget actually closes (`windowWillClose`, `menuWillClose`, `applicationWillClose` etc. messages in Cocoa).
> 
> This led to the current patch, which cycles through and closes the `Sublime::View`s associated with the window area, and then lets Qt send and process all events that are pending and/or result from closing those windows. Only then does it proceed the regular course of action (calling `removeHighLighting()` which ought to be no longer necessary).
> 
> The only change for the user is that the pinkish patch review plugin background (with the "back to code" button) is visible briefly before the toolview is closed.
> 
> I understand that this crash (or a closely related one) does not only occur on OS X but also on Linux, so I did not put the new code in a conditional block.
> 
> 
> Diffs
> -----
> 
>   plugins/grepview/grepviewplugin.cpp 7e3d933 
>   plugins/patchreview/patchreview.cpp 18b63db 
>   shell/sessioncontroller.cpp ae4e355 
>   vcs/vcspluginhelper.cpp 26ab57c 
>   vcs/widgets/vcsdiffpatchsources.cpp c3e2dae 
>   vcs/widgets/vcsdiffwidget.cpp 54787b9 
> 
> Diff: https://git.reviewboard.kde.org/r/120420/diff/
> 
> 
> Testing
> -------
> 
> kdevplatform git/kde4-legacy on KDE/MacPorts OS X 10.6.8 with kdelibs 4.14.1
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20141219/16510b41/attachment-0001.html>