Review Request 123648: Guard access to kscreen configuration

[Aleix Pol Gonzalez]

> On May 6, 2015, 12:57 a.m., Aleix Pol Gonzalez wrote:
> > LGTM, thanks for looking into this.
> > 
> > The alternative David suggests also sounds good, maybe better but not by that much.
> 
> Sebastian Kügler wrote:
>     Agree. I'll let the guard in as well, just to be super-safe that the crash is gone (and not reintroduced).
>     
>     Do you recall why we're also monitoring QScreen?

The correct screen removed signal comes from Qt, if we use the one from KScreen we're sometimes too late and it becomes a mess because Qt tries to bodge a solution.


- Aleix


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


On May 6, 2015, 12:08 a.m., Sebastian Kügler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123648/
> -----------------------------------------------------------
> 
> (Updated May 6, 2015, 12:08 a.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/09e98d8a/attachment.html>