> On Jan. 7, 2014, 3:14 p.m., Martin Gräßlin wrote: > > checking obviously makes sense, though it shouldn't be needed. There must > > be something else which is wrong here, too. > > > > Could you try what the value of WId is in these cases? I wouldn't be > > surprised if it were 0. > > > > Oh and that code has unit tests, so I would appreciate if you extend the > > tests for that case. > > David Edmundson wrote: > WId seems to be valid. If I check the dialog with xwininfo before closing > plasmoidviewer it shows the same ID. > > Here is a full backtrace of it being needed: > http://pastebin.kde.org/pxjhgw95d > > I can guard against it inside plasma with > if (!QApplication::closingDown()) around the KWindowEffects calls. > > I changed to guarding in the library as I can imagine others hitting it > in the future and in general library code shouldn't crash on reasonable > inputs. > > > > > > Martin Gräßlin wrote: > The backtrace includes QWindow::destroy which as the name says will do an > xcb_destroy_window. After that the DialogProxy::onVisibleChanged will be > invoked. At that point the window doesn't exist any more so the X calls are > just wrong. I'd say this needs fixing in DialogProxy to not call the X > specific calls for a destroyed window. > > Then one could think about whether invoking the methods without a valid > xcb_connection is supposed to work. I'd say we should add asserts there > instead of the null checks. What do you think? > > Martin Gräßlin wrote: > I just had a look at the QWindow implementation and whether it could > provide us a useable way to figure out whether there is a window at all. > Unfortunately it doesn't. So ShipIt! > > David Edmundson wrote: > I had a try at making a unit test but couldn't reproduce the crash I was > seeing in Plasma; deleting the m_widget and m_window before calling > KWindowEffect::whatever didn't seem to be enough.
I think it must be really shutting down and that might not be a testable case in the unit tests. - Martin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114895/#review46967 ----------------------------------------------------------- On Jan. 7, 2014, 2:57 p.m., David Edmundson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/114895/ > ----------------------------------------------------------- > > (Updated Jan. 7, 2014, 2:57 p.m.) > > > Review request for KDE Frameworks, Martin Gräßlin and Marco Martin. > > > Repository: kwindowsystem > > > Description > ------- > > Guard against null QX11Info::connection() > > This can fail if the application is currently shutting down, > this is currently causing a crash on closing plasma with dialogs > open. > > > Diffs > ----- > > src/kwindoweffects_x11.cpp 72cbb71 > > Diff: https://git.reviewboard.kde.org/r/114895/diff/ > > > Testing > ------- > > Opened plasmoidviewer -a org.kde.example.widgetgallery expanded the applet, > then closed plasmoidviewer > It used to crash, now it doesn't. > > > Thanks, > > David Edmundson > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
