860442 - B2G emulator reftest failures and timeouts after layers refactoring

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Andrew Halberstadt [:ahal]]

This is fallout from the layers refactoring in bug 825928. Reftests are currently so the bustage wasn't caught right away (you can see them with https://tbpl.mozilla.org/?tree=Mozilla-Inbound&showall=1&jobname=b2g)

Each chunk has many failures, and eventually times out after 330 seconds with no output, e.g:
https://tbpl.mozilla.org/php/getParsedLog.php?id=21655253&tree=Mozilla-Inbound&full=1

Comment 1

[Milan Sreckovic [:milan] (needinfo for best results)]

Assigning to Jeff to track the team effort.

Comment 2

[Jeff Muizelaar [:jrmuizel]]

This looks like an OOM

Comment 3

[Jeff Muizelaar [:jrmuizel]]

We're leaking gralloc buffers

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Attachment: Delete the gralloc buffer even when we don't have a PBufferParent

We are leaking in the snapshot path of BasicShadowLayerManager::EndTransaction.

We allocate a surface descriptor in AllocSurfaceDescriptor. However, it does not have a bufferParent and so when we call ISurfaceAllocator::PlatformDestroySharedSurface it ends up leaking.

This fixes the leak. However, I don't have much of an idea of how correct it is.

Comment 5

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

(In reply to Jeff Muizelaar [:jrmuizel] from comment #4)
> This fixes the leak. However, I don't have much of an idea of how correct it
> is.

I don't either. Could you at least describe how you came up with this idea? It wouldn't even have crossed my mind.

Let's flag jdm for needinfo: is it a typical idiom, to go "if we don't have a parent, use the child" like this patch does? What sense does it make?

Comment 6

[Josh Matthews [:jdm]]

Yes, that is a common idiom in IPC code.

Comment 7

[Josh Matthews [:jdm]]

In particular, one and only one of those methods will return a non-null value, depending on whether it's running on the parent or child.

Comment 8

[Jonathan Griffin (:jgriffin)]

There is also an intermittent crash in B2G mochitest, with a stack trace that looks like it might be relevant:

https://tbpl.mozilla.org/php/getParsedLog.php?id=21686207&tree=Mozilla-Inbound

Comment 9

[Jeff Muizelaar [:jrmuizel]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fe2fe1ad36d9

Comment 10

[Ryan VanderMeulen [:RyanVM]]

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

Comment 12

[Andrew Halberstadt [:ahal]]

There are still some perma-orange tests in R1 and R6
https://tbpl.mozilla.org/php/getParsedLog.php?id=21733320&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=21733404&tree=Mozilla-Inbound

(ignore the input-file-*.html tests, they are bug 855352)

Jeff, should I file a separate bug for these? Or are they symptoms of known issues.

Comment 13

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 736137 [details] [diff] [review]
Delete the gralloc buffer even when we don't have a PBufferParent

Nical reviewed this.