Differential D17311

Make sure that effect windows outlive effects
ClosedPublic
Actions

Authored by zzag on Dec 2 2018, 3:25 PM.

Details

Reviewers

graesslin

Group Reviewers

KWin

Commits

R108:295138145ca2: Make sure that effect windows outlive effects

Summary

Compositing is suspended/finished in a very hard way fashion, effect
windows are destroyed without notifying effects about it.

AnimationEffect tries gracefully release deleted windows, but because
in some cases(like when suspending compositing) a deleted window can
be already destroyed, a segmentation fault can happen.

This change adjusts the order in which effect windows and effects are
destroyed, so AnimationEffect (and other effects) cannot access dangling
pointers.

BUG: 400788
FIXED-IN: 5.15.0

Diff Detail

Repository

R108 KWin

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

zzag created this revision.Dec 2 2018, 3:25 PM

Restricted Application added a project: KWin. · View Herald TranscriptDec 2 2018, 3:25 PM

Restricted Application added a subscriber: kwin. · View Herald Transcript

zzag requested review of this revision.Dec 2 2018, 3:25 PM

Harbormaster completed remote builds in B5636: Diff 46714.Dec 2 2018, 3:25 PM

zzag added a dependency: D17306: [scenes/opengl] Cleanup cached decoration shadow textures when suspending compositing.Dec 2 2018, 3:26 PM

Are you sure that nothing in windowClosed and finishCompositing accesses EffectsHandler?

In D17311#370082, @graesslin wrote:

Are you sure that nothing in windowClosed and finishCompositing accesses EffectsHandler?

The only use-case I found is the destructor of SceneOpenGLShadow, but D17306 should address it.

graesslin accepted this revision.Dec 2 2018, 4:29 PM

This revision is now accepted and ready to land.Dec 2 2018, 4:29 PM

Closed by commit R108:295138145ca2: Make sure that effect windows outlive effects (authored by zzag). · Explain WhyDec 2 2018, 6:00 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

		Path
M		autotests/integration/CMakeLists.txt (1 line)
A	M	autotests/integration/dont_crash_reinitialize_compositor.cpp (150 lines)
M		composite.cpp (9 lines)
M		effects.h (9 lines)
M		effects.cpp (13 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	46714	c3241d0		Dec 2 2018, 3:25 PM	★	★
Diff 2	46729	3ad5604	R108:295138145ca2cc0b8b9d637f6071298d356084d0	Dec 2 2018, 5:59 PM	★	★

	Status	Author	Revision
	Closed	zzag	D17311 Make sure that effect windows outlive effects
	Closed	zzag	D17306 [scenes/opengl] Cleanup cached decoration shadow textures when suspending compositing