Review Request 128761: Fix crash on exit

[Peter Wu]

> On Aug. 27, 2016, 11:14 a.m., Hugo Pereira Da Costa wrote:
> > mmm. But then i think it is better (for commit history etc), to just revert the incriminated commit (and in oxygen as well), with a possible link to this RB. makes sense ?

I tested the reverts for breeze and oxygen and that also works on Qt 5.7.0 (tested with `wireshark -o` (`exit()`s), `wireshark` + quit and the testcase).

Reverted:
breeze 1430dd22ae74a09ba289dafe2be15628a0eddc15
oxygen e0376e1597f6e6b49e6343874e141b439565b749

Due to `delete qApp->style()` however, it is in theory still possible to reach a use-after-free since QApplication owns it.

I'll leave it up to you whether you take this patch or revert it, I'm fine with either.


- Peter


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


On Aug. 27, 2016, 11:12 a.m., Peter Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128761/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2016, 11:12 a.m.)
> 
> 
> Review request for Plasma, David Edmundson, David Faure, and Hugo Pereira Da Costa.
> 
> 
> Bugs: 356940
>     https://bugs.kde.org/show_bug.cgi?id=356940
> 
> 
> Repository: oxygen
> 
> 
> Description
> -------
> 
> Since Qt 5.6.0, Qt5 applications started crashing on exit. All signs
> point to this delete-on-destroy hack which was added to avoid outliving
> the plugin lifetime.
> 
> This method is wrong because the returned style is owned by the caller
> (QApplication, QProxyStyle, etc) and will cleaned up when those users
> are destructed.
> 
> Copied from breeze patch https://git.reviewboard.kde.org/r/128760/
> 
> 
> Diffs
> -----
> 
>   kstyle/oxygenstyleplugin.cpp 70b90d9 
> 
> Diff: https://git.reviewboard.kde.org/r/128761/diff/
> 
> 
> Testing
> -------
> 
> Started `QT_STYLE_OVERRIDE=oxygen LD_LIBRARY_PATH=... QT_PLUGIN_PATH=... wireshark -o` (an invalid option that triggers `exit(1)`) and observe a heap-use-after free similar to the one reported in the bug. Apply this patch, rebuild oxygen and notice that the crash is fixed. Also tested with "Testcase (ASAN)" from bug 356940, crash is also gone.
> 
> 
> Thanks,
> 
> Peter Wu
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160827/bc967e08/attachment-0001.html>