RFC: [KWindowShadows] Check for X connection
ClosedPublic

Authored by broulik on Jan 28 2020, 11:45 AM.

Details

Summary

On teardown of plasmashell the shared pointer with shadows is unref'd quite late at which point QX11Info::connection() already return null, leading to a crash in those xcb calls.
The old code in Plasma PanelShadows checked for QX11Info::display() being null, which is restored here.

Test Plan

Not sure if this is a good idea? is X smart enough to clean up after us?

  • Still crashes in Breeze style but no longer without it

Diff Detail

Repository
R278 KWindowSystem
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
broulik created this revision.Jan 28 2020, 11:45 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptJan 28 2020, 11:45 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
broulik requested review of this revision.Jan 28 2020, 11:45 AM
zzag accepted this revision.Jan 28 2020, 11:46 AM
This revision is now accepted and ready to land.Jan 28 2020, 11:46 AM

Heh, I'm going to quote a conversation with you:


David Edmundson, [17.01.20 10:07]
would you say it's always wrong to have a QWindow outlive a QApplication?

Kai Uwe, [17.01.20 10:08]
I would say yes, why?

David Edmundson, [17.01.20 10:08]
wondering if I need to comment on some code that would crash if that happened

Kai Uwe, [17.01.20 10:13]
when qapp is gone the qpt and stuff will be torn down and then it will naturally crash if a window outlives it, no?

Then you went and proved that windows are torn down.


If we have a tile alive it means either a KWindowShadow object is alive or the QStyle is still alive, and both should be destroyed before the QPA.
Do you have the full bt of the crash?

davidedmundson accepted this revision.Jan 28 2020, 12:00 PM

Ok, panel shadows is a singleton. That explains things.

This revision was automatically updated to reflect the committed changes.