Review Request 128040: [shellcorona] Containment can be nullptr

[David Edmundson]

> On May 30, 2016, 11:32 a.m., Aleix Pol Gonzalez wrote:
> > Could you look a bit into why is the containment null?
> 
> Anthony Fieroni wrote:
>     So i can speculate, because i have 2 Samsung TV, 1 smart, 1 LCD and i can't reproduce crash, KScreen is not finish recognazing, which i write above, corona.cpp:211 if (screen >= 0 && screen < numScreens()) is not satisfied for some reason and returning containment is nullptr. I can't figure out any other possible way.
> 
> Aleix Pol Gonzalez wrote:
>     But `createContainmentForActivity` should return a containment nevertheless, it's not related to KScreen after all.
> 
> Marco Martin wrote:
>     so may the issue go away with the port to qscreen?
> 
> Anthony Fieroni wrote:
>     createContainmentForActivity calls containmentForScreen() from corona which use numScreen() overridden in shellCorona and depend on KScreen configuration. So for me is KScreen related.
> 
> David Edmundson wrote:
>     >    so may the issue go away with the port to qscreen?
>     
>     It should do.
>     
>     It's a continuation of https://bugs.kde.org/show_bug.cgi?id=351777 
>     
>     there I list the events which cause the crash:
>     
>     >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
>     
>     If Qt proceses the DBus event before we get the X event. You're out of sync.
>     (note they weren't in Plasma 5.0,it changed in ~5.2 when the XCB backend for kscreen was split into a separate process)
>     
>     I fixed the case of them being out of sync at startup which was the main time we got the crash, but it still apparently happens on unplugging/plugging in a monitor.
>     
>     As for this patch; sure you'll stop a crash, but you'll leave those setups that would have crashed with a blank screen on the monitor they've just plugged in.
> 
> Anthony Fieroni wrote:
>     *but you'll leave those setups that would have crashed with a blank screen*
>     
>     Are you sure about that? So i'm not, still think KScreen has a false positive signals, i.e. he will be done with screen recognazing

doesn't really matter: https://git.reviewboard.kde.org/r/125451/ is submitted and fixes the root cause.


- David


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


On May 30, 2016, 4:33 a.m., Anthony Fieroni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128040/
> -----------------------------------------------------------
> 
> (Updated May 30, 2016, 4:33 a.m.)
> 
> 
> Review request for Plasma, Aleix Pol Gonzalez and David Edmundson.
> 
> 
> Bugs: 361548
>     https://bugs.kde.org/show_bug.cgi?id=361548
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> I can't reproduce this tons of bug reports and i event can't undertaind how containment can be nullptr, when i see corona.cpp http://api.kde.org/frameworks/plasma-framework/html/corona_8cpp_source.html#l00447
> 
> 
> Diffs
> -----
> 
>   shell/shellcorona.cpp 2a78ca9 
> 
> Diff: https://git.reviewboard.kde.org/r/128040/diff/
> 
> 
> Testing
> -------
> 
> To stop crash ?!
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160531/8b0abdd4/attachment-0001.html>