1331761 - Crash when killing the GPU process during video decoding

Status: VERIFIED FIXED

(Core :: Graphics: Layers, defect, P1)

Description

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

+++ This bug was initially created as a clone of Bug #1331548 +++

STR:
1. Open https://www.youtube.com/watch?v=iNJdPyoqt8U in a window.
2. Open about:support in a second window.
3. While the video is playing in 4k, click the "Terminate" button for the GPU process in about:support.

It looks like we are trying to create a GPUVideoTextureHost in the new in-process compositor, which attempts to access the non-existent VideoBridgeParent singleton.

Comment 1

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

Does this happen only when GPU process video decoding is enabled, or regardless of that setting?

Comment 2

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

(In reply to Milan Sreckovic [:milan] from comment #1)
> Does this happen only when GPU process video decoding is enabled, or
> regardless of that setting?

Never mind, answered in bug 1331548.

Comment 3

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

Matt suggested a fix that seems very reasonable to me - protect ImageClient from sending stale TextureClients to the compositor. But I can't reproduce this anymore and have been trying on both debug/opt on multiple machines. I also don't see any crashes involving VideoBridgeParent on crash-stats.

I'm loathe to attempt a fix without any way to see that it works, so I'm holding off for now.

Comment 4

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

Perhaps we can have a MOZ_ASSERT added in the cases that this would fix, and see if anybody on nightly hits it?  Then replace the assert with the fix.

Comment 5

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

I spoke too soon... this morning I can reproduce it about 30% of the time.

Comment 6

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

Attachment: fix

Am able to verify now that this does fix the issue.

Comment 7

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8832618 [details] [diff] [review]
fix

Review of attachment 8832618 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/client/ImageClient.cpp
@@ +229,5 @@
> +    // We check if the texture's allocator is still open, since in between media
> +    // decoding a frame and adding it to the compositable, we could have
> +    // restarted the GPU process.
> +    if (!texture || !texture->GetAllocator()->IPCOpen()) {
> +      return false;

Can we continue and see if any of the other textures in the list work?

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d91ebcfb7bbb
Don't send stale GPUVideoTextures to in-process compositors. (bug 1331761, r=mattwoodrow)

Comment 9

[Carsten Book [:Tomcat]]

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

Comment 10

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

Comment on attachment 8832618 [details] [diff] [review]
fix

Approval Request Comment
[Feature/Bug causing the regression]: GPU Process
[User impact if declined]: potential browser crashes if the GPU process dies
[Is this code covered by automated tests?]:
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]:
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: This is a simple check that we are not using a stale IPC channel to send video. It's baked on aurora and nightly.
[String changes made/needed]:

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8832618 [details] [diff] [review]
fix

Fix for a graphics crash, let's take it on beta.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c92e96316678

Comment 14

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

I reproduced this issue using Fx 53.0b8 on Windows 10 x64.
I can confirm this issue is fixed, I verified using Fx 54.0a2 (build ID: 20170403004002) and Fx 53.0b9 on Windows 10 x64.

Cheers!