Avoid deleting BlurInterface global
Changes PlannedPublic

Authored by davidedmundson on Apr 16 2020, 3:00 PM.

Details

Reviewers
None
Group Reviewers
KWin
Summary

If a global is deleted whilst a client is trying to bind, the wayland
server will still dispatch the event - there will be no object to
dispatch to and the client will be killed.

Effects are torn down and recreated at runtime. Often in quick
succession which makes this easy to hit.

The use of a static isn't ideal but it keeps the encapsulation.

BUG: 419912

Test Plan

Changed fonts, plasma didn't crash

Diff Detail

Repository
R108 KWin
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 25322
Build 25340: arc lint + arc unit
davidedmundson created this revision.Apr 16 2020, 3:00 PM
Restricted Application added a project: KWin. · View Herald TranscriptApr 16 2020, 3:00 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
davidedmundson requested review of this revision.Apr 16 2020, 3:00 PM

Patch will be copy pasted for ContrastInterface if approved

davidedmundson edited the summary of this revision. (Show Details)Apr 16 2020, 3:00 PM
davidedmundson retitled this revision from Avoid deleting BlueInterface global to Avoid deleting BlurInterface global.
apol added a subscriber: apol.Apr 16 2020, 5:09 PM
apol added inline comments.
effects/blur/blur.h
134

Why the rename? it's not static...

zzag added a subscriber: zzag.EditedApr 17 2020, 7:32 AM

At some point, we need to call wl_global_destroy; otherwise we'll be leaking globals.

anthonyfieroni added inline comments.
effects/blur/blur.cpp
61

delete manager when display is going to die.

effects/blur/blur.h
134

It should be static.

anthonyfieroni added inline comments.Apr 17 2020, 7:43 AM
effects/blur/blur.cpp
61

It looks like it's done by parent the creation.

davidedmundson planned changes to this revision.May 11 2020, 6:39 PM

blocked due to distros not having a new kwayland :(

Will try again in a month.

and yes, I meant to make it static - the fix still works without but means we keep "leaking" blur resources every time we add and remove