Differential D6569

[client] Track all created ConnectionThreads and add API to access them
ClosedPublic
Actions

Authored by graesslin on Jul 8 2017, 12:37 PM.

Details

Reviewers
davidedmundson
Group Reviewers
Frameworks
Plasma
KWin
Commits
R127:e4c90bf90717: [client] Track all created ConnectionThreads and add API to access them
Summary

This is change needed by KWin. KWin has the problem that it destroys its
internal Wayland connection (KWin as client for KWin as server) before
shutting down the application. Other external libraries loaded into KWin
(e.g. breeze window decoration) are unloaded later on, then try to clean
up their Wayland resources and crash KWin due to accessing a no longer
valid Wayland connection.

With the help of this new API KWin can access all connections during
the clean up and destroy them before shutting down the Wayland server and
thus exit cleanly.

Diff Detail

Repository
R127 KWayland
Branch
all-connection-threads
Lint
No Linters Available
Unit
No Unit Test Coverage
graesslin created this revision.Jul 8 2017, 12:37 PM
Restricted Application added projects: Plasma on Wayland, Frameworks. · View Herald TranscriptJul 8 2017, 12:37 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
dfaure added a subscriber: dfaure.Jul 8 2017, 1:29 PM

Isn't a mutex needed to protect this data structure that is apparently modified by different threads?

davidedmundson added a subscriber: davidedmundson.Jul 8 2017, 2:19 PM

I don't think this is necessary.

The only times we want need to delete the connection ourselves is when client code is creating it via ConnectionThread::fromApplication. If they create it themselves, we don't want to, and there's no need to track it.

For the one instance in the bug you're fixing we have the ConnectionThread object already in kwin as it's in kwin's own QPA.

graesslin added a comment.Jul 8 2017, 7:40 PM
In D6569#123081, @davidedmundson wrote:

I don't think this is necessary.

The only times we want need to delete the connection ourselves is when client code is creating it via ConnectionThread::fromApplication. If they create it themselves, we don't want to, and there's no need to track it.

For the one instance in the bug you're fixing we have the ConnectionThread object already in kwin as it's in kwin's own QPA.

The problem is not the one KWin creates, but the one Breeze creates and the one the KWindowSystem plugin creates and the one plasma-integration and so on and on...

graesslin added a comment.Jul 9 2017, 1:01 PM

@davidedmundson if you have another idea how to tackle the problem I'm absolutely open to it.

davidedmundson accepted this revision.Jul 9 2017, 3:16 PM
This revision is now accepted and ready to land.Jul 9 2017, 3:16 PM
graesslin updated this revision to Diff 16406.Jul 9 2017, 7:39 PM

Use mutex to protect the potentially shared data structure

Restricted Application edited projects, added Plasma; removed Plasma on Wayland. · View Herald TranscriptJul 9 2017, 7:39 PM
graesslin updated this revision to Diff 16660.Jul 13 2017, 3:38 PM

Make this compile. Looks like kdevelop fooled me and compiled KWin instead of KWayland (that happens all the time to me)

Restricted Application edited projects, added Plasma on Wayland; removed Plasma. · View Herald TranscriptJul 13 2017, 3:38 PM
Closed by commit R127:e4c90bf90717: [client] Track all created ConnectionThreads and add API to access them (authored by graesslin). · Explain WhyJul 21 2017, 5:15 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 16406

View Options

autotests/client/test_wayland_connection_thread.cpp

Loading...
View Options

src/client/connection_thread.h

Loading...
View Options

src/client/connection_thread.cpp

Loading...