Delete all Wayland connections by plugins prior to own Wayland connection
ClosedPublic

Authored by graesslin on Jul 8 2017, 1:25 PM.

Details

Summary

Some libraries loaded into KWin (e.g. breeze) also have a Wayland
connection. If KWin destroyes it's own Wayland connection before the
libraries destroy theirs, KWin might crash on tear down when the
libraries call into libwayland with an invalid connection.

This change requires D6569 in KWayland.

Diff Detail

Repository
R108 KWin
Branch
connection-cleanup
Lint
No Linters Available
Unit
No Unit Test Coverage
graesslin created this revision.Jul 8 2017, 1:25 PM
Restricted Application added a project: KWin. · View Herald TranscriptJul 8 2017, 1:25 PM
Restricted Application added subscribers: kwin, plasma-devel. · View Herald Transcript
mart accepted this revision.Jul 11 2017, 2:59 PM
This revision is now accepted and ready to land.Jul 11 2017, 2:59 PM
davidedmundson added inline comments.
wayland_server.cpp
93

s/hold/held.

99

Alternative.

Instead of deleting qobjects

emit c->connectionDied();

Clients then call destroy() on whatever objects.
They can then delete the QObjects whenever they were going to.

graesslin added inline comments.Jul 15 2017, 7:56 PM
wayland_server.cpp
99

Yeah that should work as well. Though of course the documentation of ConnectionThread says it's not emitted if created from application, but we can fix that ;-)

It might mean we leak a few resources, but as we are shutting down anyway...

davidedmundson added inline comments.Jul 27 2017, 5:37 PM
wayland_server.cpp
99

RE: D6727

That would basically allow us to abandon D6571 and would also solve the issues we see with the kwayland-integration plugin which can crash applications on exit.

I'd like to see this patch merged (but with the emit connectionDied)
It might work without, but it still leaves a window where someone calling foo->destroy() would crash.

graesslin updated this revision to Diff 17299.Jul 27 2017, 6:15 PM

Updated to master and emit of connectionDied instead of delete

Restricted Application edited projects, added Plasma; removed KWin. · View Herald TranscriptJul 27 2017, 6:15 PM
davidedmundson accepted this revision.Jul 28 2017, 9:19 AM
This revision was automatically updated to reflect the committed changes.

Good news: the combo worked! The crashing useractionsmenu test just passed on build.kde.org.