> On Aug. 27, 2016, 9: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 ? > > Peter Wu wrote: > 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.
i agree, obviously delete qApp->Style() must go too, and in fact that was the original patch introduced by D Faure, to oxygen (commit 2ffe20e1bfe93c921c5372b4d21447b1de308d4b), which i then copied to breeze with many other changes and wich was later on changed to the signal/slot delta function by D Edmundson. So in the end, maybe it is indeed better to clean it all up in one go, possibly adding the relevant effectively reverted commits in the comments. - Hugo ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128761/#review98709 ----------------------------------------------------------- On Aug. 27, 2016, 9: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, 9: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 > >
