Review Request 126961: Fix the infamous Plasma::Applet::Actions crash

Sebastian Kügler sebas at kde.org
Tue Feb 2 12:43:36 UTC 2016


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




shell/shellcorona.cpp (line 629)
<https://git.reviewboard.kde.org/r/126961/#comment62733>

    m_screenConfiguration may still be a nullptr here, since it's set only after GetConfigOperation returns.
    
    That may well be the reason why the sync QScreen's count is used here, but this leads to problems down the road, as you note.
    
    We could make sure we have the number of screens returned here by making GetConfigOperation sync, so using its exec() and thus blocking in the setShell call.


- Sebastian Kügler


On Feb. 1, 2016, 11:08 p.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126961/
> -----------------------------------------------------------
> 
> (Updated Feb. 1, 2016, 11:08 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> We were mixing KScreen and QScreen API badly.
> 
> Corona.cpp checks we are requesting a containment for a valid screen
> if (screen >= 0 && screen < numScreens()) {
> 
> This fails as numScreens() is Qt API based, whereas the signal we're
> adding the output for is ShellCorona::addOutput so we have an effective race condition.
> 
> BUG: 351777
> 
> 
> Diffs
> -----
> 
>   shell/shellcorona.cpp 762e503bf59fe648fb0f5b76a36229aa43c563e5 
> 
> Diff: https://git.reviewboard.kde.org/r/126961/diff/
> 
> 
> Testing
> -------
> 
> Started Plasma on dual screen.
> 
> Ideally we need to do more testing before backporting, as that entire codebase is a disgrace.
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

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


More information about the Plasma-devel mailing list