967989 - Faulty: main process hang with main thread stuck in PCompositorChild::SendFlushRendering after non-fatal assert failures "aChild already in the tree" and "aChild already has siblings?"

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: Faulty session

Found by Christoph Diehl's "Faulty" fuzzer, see bug 777067

Hit while running css-gradients reftest.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

The log has these just before the hang:

[Parent 3649] ###!!! ASSERTION: aChild already in the tree: '!aChild->GetParent()', file /hack/mozilla-central/gfx/layers/Layers.cpp, line 724
[Parent 3649] ###!!! ASSERTION: aChild already has siblings?: '!aChild->GetNextSibling() && !aChild->GetPrevSibling()', file /hack/mozilla-central/gfx/layers/Layers.cpp, line 726

This implies a bad layer tree is being built, in which case it's totally expected that UpdatePanZoomControllerTree will go off into the weeds and never come back. We should track down why those assertions are getting tripped.

There was a similar issue recently in bug 950301 (see comment 15 there) where a bad layer tree was being built and causing an infinite loop in UpdatePanZoomControllerTree. The reason it happens there rather than other places is because we walk the tree backwards which other code doesn't do, so we exercise a different set of links than most.

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

If failure of these assertions can lead to hangs, would it then be reasonable to make these assertions fatal, so we can more easily understand why they fail?

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

Actually, I just reproduced a similar hang, still with the main thread stuck in PCompositorChild::SendFlushRendering, still just after failing these assertions, but now the compositor thread is in a different place, probably meaning that APZC is a red herring. Attaching new session...

Comment 4

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: Another Faulty session. Compositor in a different place, but same assert failures and main thread in same place.

Comment 5

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: make ContainerLayer::InsertAfter assertions fatal to get a more helpful crash

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Benoit Jacob [:bjacob] from comment #2)
> If failure of these assertions can lead to hangs, would it then be
> reasonable to make these assertions fatal, so we can more easily understand
> why they fail?

I don't see how that would help understand why they fail.

Comment 7

[Benoit Jacob [:bjacob] (mostly away)]

That would conceivably help as we would then get a stack trace when they fail. However, I have made that change locally so I will get that the next time that I hit this bug. Nothing needs to land here.

Comment 8

[Benoit Jacob [:bjacob] (mostly away)]

Fixed by the landing of shadow layer updates validation fixes, bug 970747.