Review Request 122184: Remove seemingly outdated hack deleting the currently used QStyle at teardown

Hugo Pereira Da Costa hugo.pereira at free.fr
Wed Jan 21 15:38:21 UTC 2015


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


the delete call was introduces by D. Faure, at 2ffe20e1bfe93c921c5372b4d21447b1de308d4b
with log message:

Fix crash on exit in all QCommandLineParser-based programs.
    
    Example: kioclient5 ls  (= a syntax error).
    
    The issue is that ::exit(1) is called, so ~QGuiApplication isn't called
    (so the style isn't deleted), but global static objects are deleted,
    which deletes the style-plugin-factory, which unloads the style plugin.
    
    The crash happened because "AppEventFilter" in oxygen would still be
    installed as an app event filter, but the plugin was unloaded, so any
    calls to it would crash.
    
can you double-check ?
I'll add it to the review.

- Hugo Pereira Da Costa


On Jan. 21, 2015, 3:34 p.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122184/
> -----------------------------------------------------------
> 
> (Updated Jan. 21, 2015, 3:34 p.m.)
> 
> 
> Review request for Plasma and Hugo Pereira Da Costa.
> 
> 
> Repository: oxygen
> 
> 
> Description
> -------
> 
> According to the comment we need to delete the qstyle object before the
> plugin if the application exits with exit(). A small test showed this is
> no longer the case.
> 
> More importantly this patch fixes a crash we see when styles were switched at
> runtime and the app uses exit().
> 
> Because Breeze has a similar delete line the first would run deleting
> the style object but leaving a dangling pointer, the second would run
> and crash.
> 
> Even if they only deleted their own style, both need to check the active style which means the second one is always accessing a dangling pointer.
> 
> ----
> 
> A similar approach needs to go into Breeze too. I shall copy that if this gets in.
> 
> 
> Diffs
> -----
> 
>   kstyle/oxygenstyleplugin.cpp 409ec13 
> 
> Diff: https://git.reviewboard.kde.org/r/122184/diff/
> 
> 
> Testing
> -------
> 
> Wrote a unit test that calls exit(0) on button press. This used to crash if we changed theme and then pressed the button. Now it no longer crashes ever.
> 
> Checked that kglobalaccel5 no longer crashes on exit after changing Breeze->Oxygen and does not crash after changing from Oxygen->Breeze
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20150121/116d16d7/attachment.html>


More information about the Plasma-devel mailing list