[kstyle] Unregister a shadow when it's destroyed
ClosedPublic

Authored by zzag on Jan 28 2020, 2:59 PM.

Details

Summary

Since lifetime of a KWindowShadow doesn't strictly match the lifetime
of the associated widget, we need to unregister the shadow when it's
destroyed in order to prevent accessing or deleting dangling pointers
afterwards.

BUG: 416854

Test Plan

plasmashell no longer crashes.

Diff Detail

Repository
R31 Breeze
Branch
fix-crash
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 21742
Build 21760: arc lint + arc unit
zzag created this revision.Jan 28 2020, 2:59 PM
Restricted Application added a project: Plasma. · View Herald TranscriptJan 28 2020, 2:59 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
zzag requested review of this revision.Jan 28 2020, 2:59 PM
apol added a subscriber: apol.Jan 28 2020, 3:06 PM
apol added inline comments.
kstyle/breezeshadowhelper.h
157 ↗(On Diff #74497)

if this is a thing, maybe it would make sense to actually try and remove the KWindowShadows upon destruction?
Something like connect(newshadow, &QObject::destroyed, this, [this, widget] { _shadows.remove(widget); });

broulik added inline comments.Jan 28 2020, 3:09 PM
kstyle/breezeshadowhelper.h
157 ↗(On Diff #74497)

We have a connection to destroyed for the widget which isn't reached when a window is destroyed on QApplication teardown, so I think the same might be true for the window, I would still prefer this over a QPointer.

zzag updated this revision to Diff 74502.Jan 28 2020, 3:14 PM

Connect to QObject::destroyed

zzag added inline comments.Jan 28 2020, 3:19 PM
kstyle/breezeshadowhelper.h
157 ↗(On Diff #74497)

if this is a thing, maybe it would make sense to actually try and remove the KWindowShadows upon destruction?

Yes, that's a better idea.

157 ↗(On Diff #74497)

so I think the same might be true for the window

No, it works fine.

zzag updated this revision to Diff 74504.Jan 28 2020, 3:53 PM
zzag retitled this revision from [kstyle] Use guarded pointers to store shadows to [kstyle] Unregister a shadow when it's destroyed.
zzag edited the summary of this revision. (Show Details)

Re-title and edit summary.

+1
Can confirm that the QWindow belonging to KlipperPopup that caused the crash before is destroyed before the destructor, fixing the crash.

zzag updated this revision to Diff 74506.Jan 28 2020, 4:00 PM
zzag edited the summary of this revision. (Show Details)

Edit summary.

This revision was not accepted when it landed; it landed in state Needs Review.Jan 28 2020, 4:12 PM
This revision was automatically updated to reflect the committed changes.