Review Request 123648: Guard access to kscreen configuration

[David Edmundson]

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

Ship it!


Good stuff :)


shell/shellcorona.cpp (line 189)
<https://git.reviewboard.kde.org/r/123648/#comment54779>

    an alternate solution would also be to move this to be inside ::load()
    
    soo after GetConfigOperation is done.
    
    depends if we prefer error handling or avoiding having the error in the first place. I don't have a strong opinion, your choice.


- David Edmundson


On May 5, 2015, 10:08 p.m., Sebastian Kügler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123648/
> -----------------------------------------------------------
> 
> (Updated May 5, 2015, 10:08 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 346590
>     https://bugs.kde.org/show_bug.cgi?id=346590
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> Guard access to kscreen configuration
> 
> reconsiderOutputs() may be called before the async config fetching
> operation (KScreen::GetConfigOperation) has finished, so the pointer is
> still null at that point. This can happen if qApp::screenRemoved fires
> before kscreen was able to start the backend loader and pass back a
> ConfigPtr.
> 
> The bug is rooted by in the dual-use of libkscreen and qApp's QScreen
> handling. The timings here produce this race condition. This patch
> should be quite safe, though, since it doesn't change the logic, but
> rather makes sure this race condition bails out nicely by falling back
> to just waiting until a kscreen config has arrived (which will happen
> right after).
> 
> BUG:346590
> REVIEW:
> FIXED-IN:5.3.1
> 
> 
> Diffs
> -----
> 
>   shell/shellcorona.cpp 558788daad66f8be7cc35955a5600ddab655cde6 
> 
> Diff: https://git.reviewboard.kde.org/r/123648/diff/
> 
> 
> Testing
> -------
> 
> I can't reproduce this crash myself.
> 
> 
> Thanks,
> 
> Sebastian Kügler
> 
>

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