> 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 > >
