1297567 - ClientContainerLayer is missing a Disconnect override

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[David Anderson [:dvander] - inactive, e-mail if emergency]

When the GPU process dies, the top-level actors are notified as soon as IPDL detects an erroneous state. Then, IPDL will recursively destroy all actors in the channel, in depth-first order.

What this means is that PLayerTransaction children could be destroyed outside of Layers. For example:

http://searchfox.org/mozilla-central/rev/b38dbd1378cea4ae83bbc8a834cdccd02bbc5347/gfx/layers/client/ClientLayerManager.cpp#883

This will crash because the actor has already been destroyed. Normally we would solve this by having IPDL own a reference rather than full ownership. But it could be that we want to have the actor sweep layers instead.

Nical, what do you think?

Comment 1

[Nicolas Silva [:nical]]

Usually we handle this by clearing references in ActorDestroy before they become dangling.
ShadowLayerChild::ActorDestroy at least tries to do something like that but perhaps the situation you are describing ends up calling ActorDestroy with AncestorDeletion rather than AbnormalShutdown?

Since PLayer is a simple actor with not shared state, I think that just clearing references in ActorDestroy is the simplest solution, this way, layer code only has to check HasShadow() instead of HasShadow() && DidItShutdownOnMeAlready(). But since PLayer is simple I don't have a very strong opinion.

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix

You're right - one of the Disconnect methods ended up being missing.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd4f2955a72f
Fix dangling IPDL pointer in ClientContainerLayer. (bug 1297567, r=nical)

Comment 4

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for build bustage:

https://hg.mozilla.org/integration/mozilla-inbound/rev/68b1f2438565eff2b8b284c39fb6aaeffbab7471

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=dd4f2955a72f8f942d2aabec6d0fedb35d167519

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=34658292&repo=mozilla-inbound
> /builds/slave/m-in-m64-000000000000000000000/build/src/gfx/layers/client/ClientContainerLayer.h:136:16: error: 'Disconnect' overrides a member function but is not marked 'override' [-Werror,-Winconsistent-missing-override]

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ba7cb37c582a
Fix dangling IPDL pointer in ClientContainerLayer. (bug 1297567, r=nical)

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ba7cb37c582a