Review Request 128016: [OS X] Prevent a crash in the IdealDockWidget's context menu

[René J.V. Bertin]

> On Aug. 2, 2017, 12:15 p.m., Friedrich W. H. Kossebau wrote:
> > René, can you tell why exactly this QMenu instance needs this delayed deletion? There are other QMenus created on the stack elsewhere in KDevelop code, even Qt code has QMenus on the stack (e.g. static QMenu::exec(QList<QAction*> actions, const QPoint &pos, QAction *at, QWidget *parent) implementation), so being deleted in sync (and others for some reason are on the heap, yet still deleted in-sync).
> > 
> > So why it this needed here, but not elsewhere? That information is missing here, and thus this code will be some "magic" fix to an unknown specific problem, which as a results decreases code maintainability. There should be at least a comment in the code, why this extra treatment is done here.

I would have in the original summary if I could. This must have been related to events that were pending or somehow generated because of ongoing activity and that were delivered to the QMenu instance after its destruction. As you know, the underlying Cocoa APIs all use object management based on reference counting that has become increasingly automatic (= developer need not bother with it). That can make it tricky to integrate with a C++ wrapper. I would expect that Qt has been tested much more extensively on Mac and by seasoned Mac developers (= the parts where it doesn't use delayed deletion despite their own warning).

Either way it seems that I haven't been needing the patch for a while now.


- René J.V.


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


On May 26, 2016, 5:34 p.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128016/
> -----------------------------------------------------------
> 
> (Updated May 26, 2016, 5:34 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> OS X can be capricious when instances corresponding to a widget are deleted, if the class in question uses "native" ObjC SDKs behind the scenes. Pending events can in that case be (generated and) delivered to objects that were already deleted.
> According to the documentation, one should prefer to use `QObject::deleteLater()` rather than the regular, direct `delete` whether it be explicit or implicit.
> 
> I've long used a local patch that uses this approach in order to prevent a recurring crash after using the context menu of the "ideal dock widget". Somehow I never put it up for review here, apparently.
> 
> 
> Diffs
> -----
> 
>   sublime/idealdockwidget.cpp dae0ea2 
> 
> Diff: https://git.reviewboard.kde.org/r/128016/diff/
> 
> 
> Testing
> -------
> 
> Builds and permits reliable behaviour on both OS X and Linux.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

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