1831099 - Deadlock in content process following GPU process crash while playing video

Status: RESOLVED FIXED

(Core :: Graphics, defect, P1)

Description

[Jamie Nicol [:jnicol]]

Easy to reproduce on Android, not sure about other platforms:

1. Start playing a youtube video
2. adb shell run-as org.mozilla.geckoview_example killall org.mozilla.geckoview_example:gpu
3. Deadlock.

Sometimes the page is not rendered at all following the GPU process restart, and remains completely white. In this case the main thread backtrace looks like:

     mozilla::layers::ImageBridgeChild::CreateImageClient(mozilla::layers::CompositableType, mozilla::layers::ImageContainer *) ImageBridgeChild.cpp:636
     mozilla::layers::ImageContainer::EnsureImageClient() ImageContainer.cpp:167
     mozilla::layers::ImageContainer::GetAsyncContainerHandle() ImageContainer.cpp:381
     mozilla::layers::WebRenderImageData::CreateAsyncImageWebRenderCommands(mozilla::wr::DisplayListBuilder &, mozilla::layers::ImageContainer *, const mozilla::layers::StackingContextHelper &, const mozilla::gfx::RectTyped<…> &, const mozilla::gfx::RectTyped<…> &, mozilla::VideoInfo::Rotation, const mozilla::wr::ImageRendering &, const mozilla::wr::MixBlendMode &, bool) WebRenderUserData.cpp:214
     mozilla::layers::WebRenderCommandBuilder::CreateImageKey(mozilla::nsDisplayItem *, mozilla::layers::ImageContainer *, mozilla::wr::DisplayListBuilder &, mozilla::wr::IpcResourceUpdateQueue &, mozilla::wr::ImageRendering, const mozilla::layers::StackingContextHelper &, mozilla::gfx::IntSizeTyped<…> &, const mozilla::Maybe<…> &) WebRenderCommandBuilder.cpp:2226
     mozilla::layers::WebRenderCommandBuilder::PushImage(mozilla::nsDisplayItem *, mozilla::layers::ImageContainer *, mozilla::wr::DisplayListBuilder &, mozilla::wr::IpcResourceUpdateQueue &, const mozilla::layers::StackingContextHelper &, const mozilla::gfx::RectTyped<…> &, const mozilla::gfx::RectTyped<…> &) WebRenderCommandBuilder.cpp:2251
     mozilla::nsDisplayVideo::CreateWebRenderCommands(mozilla::wr::DisplayListBuilder &, mozilla::wr::IpcResourceUpdateQueue &, const mozilla::layers::StackingContextHelper &, mozilla::layers::RenderRootStateManager *, mozilla::nsDisplayListBuilder *) nsVideoFrame.cpp:640
     mozilla::layers::WebRenderCommandBuilder::CreateWebRenderCommands(mozilla::nsDisplayItem *, mozilla::wr::DisplayListBuilder &, mozilla::wr::IpcResourceUpdateQueue &, const mozilla::layers::StackingContextHelper &, mozilla::nsDisplayListBuilder *) WebRenderCommandBuilder.cpp:1859
     mozilla::layers::WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList(mozilla::nsDisplayList *, mozilla::nsDisplayItem *, mozilla::nsDisplayListBuilder *, const mozilla::layers::StackingContextHelper &, mozilla::wr::DisplayListBuilder &, mozilla::wr::IpcResourceUpdateQueue &, bool) WebRenderCommandBuilder.cpp:2125
     [Inlined] mozilla::nsDisplayWrapList::CreateWebRenderCommandsNewClipListOption(mozilla::wr::DisplayListBuilder &, mozilla::wr::IpcResourceUpdateQueue &, const mozilla::layers::StackingContextHelper &, mozilla::layers::RenderRootStateManager *, mozilla::nsDisplayListBuilder *, bool) nsDisplayList.cpp:4673
     [Inlined] mozilla::nsDisplayWrapList::CreateWebRenderCommands(mozilla::wr::DisplayListBuilder &, mozilla::wr::IpcResourceUpdateQueue &, const mozilla::layers::StackingContextHelper &, mozilla::layers::RenderRootStateManager *, mozilla::nsDisplayListBuilder *) nsDisplayList.h:4988
     mozilla::nsDisplayOwnLayer::CreateWebRenderCommands(mozilla::wr::DisplayListBuilder &, mozilla::wr::IpcResourceUpdateQueue &, const mozilla::layers::StackingContextHelper &, mozilla::layers::RenderRootStateManager *, mozilla::nsDisplayListBuilder *) nsDisplayList.cpp:5312
     mozilla::nsDisplayFixedPosition::CreateWebRenderCommands(mozilla::wr::DisplayListBuilder &, mozilla::wr::IpcResourceUpdateQueue &, const mozilla::layers::StackingContextHelper &, mozilla::layers::RenderRootStateManager *, mozilla::nsDisplayListBuilder *) nsDisplayList.cpp:5509
     mozilla::layers::WebRenderCommandBuilder::CreateWebRenderCommands(mozilla::nsDisplayItem *, mozilla::wr::DisplayListBuilder &, mozilla::wr::IpcResourceUpdateQueue &, const mozilla::layers::StackingContextHelper &, mozilla::nsDisplayListBuilder *) WebRenderCommandBuilder.cpp:1859
     mozilla::layers::WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList(mozilla::nsDisplayList *, mozilla::nsDisplayItem *, mozilla::nsDisplayListBuilder *, const mozilla::layers::StackingContextHelper &, mozilla::wr::DisplayListBuilder &, mozilla::wr::IpcResourceUpdateQueue &, bool) WebRenderCommandBuilder.cpp:2125
     [Inlined] mozilla::nsDisplayWrapList::CreateWebRenderCommandsNewClipListOption(mozilla::wr::DisplayListBuilder &, mozilla::wr::IpcResourceUpdateQueue &, const mozilla::layers::StackingContextHelper &, mozilla::layers::RenderRootStateManager *, mozilla::nsDisplayListBuilder *, bool) nsDisplayList.cpp:4673
     [Inlined] mozilla::nsDisplayWrapList::CreateWebRenderCommands(mozilla::wr::DisplayListBuilder &, mozilla::wr::IpcResourceUpdateQueue &, const mozilla::layers::StackingContextHelper &, mozilla::layers::RenderRootStateManager *, mozilla::nsDisplayListBuilder *) nsDisplayList.h:4988
     mozilla::nsDisplayOwnLayer::CreateWebRenderCommands(mozilla::wr::DisplayListBuilder &, mozilla::wr::IpcResourceUpdateQueue &, const mozilla::layers::StackingContextHelper &, mozilla::layers::RenderRootStateManager *, mozilla::nsDisplayListBuilder *) nsDisplayList.cpp:5312
     mozilla::layers::WebRenderCommandBuilder::CreateWebRenderCommands(mozilla::nsDisplayItem *, mozilla::wr::DisplayListBuilder &, mozilla::wr::IpcResourceUpdateQueue &, const mozilla::layers::StackingContextHelper &, mozilla::nsDisplayListBuilder *) WebRenderCommandBuilder.cpp:1859
     mozilla::layers::WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList(mozilla::nsDisplayList *, mozilla::nsDisplayItem *, mozilla::nsDisplayListBuilder *, const mozilla::layers::StackingContextHelper &, mozilla::wr::DisplayListBuilder &, mozilla::wr::IpcResourceUpdateQueue &, bool) WebRenderCommandBuilder.cpp:2125
     mozilla::layers::WebRenderCommandBuilder::BuildWebRenderCommands(mozilla::wr::DisplayListBuilder &, mozilla::wr::IpcResourceUpdateQueue &, mozilla::nsDisplayList *, mozilla::nsDisplayListBuilder *, mozilla::layers::WebRenderScrollData &, WrFiltersHolder &&) WebRenderCommandBuilder.cpp:1780
     mozilla::layers::WebRenderLayerManager::EndTransactionWithoutLayer(mozilla::nsDisplayList *, mozilla::nsDisplayListBuilder *, WrFiltersHolder &&, mozilla::layers::WebRenderBackgroundData *, double) WebRenderLayerManager.cpp:363
     mozilla::nsDisplayList::PaintRoot(mozilla::nsDisplayListBuilder *, gfxContext *, unsigned int, mozilla::Maybe<…>) nsDisplayList.cpp:2342
     nsLayoutUtils::PaintFrame(gfxContext *, nsIFrame *, const nsRegion &, unsigned int, mozilla::nsDisplayListBuilderMode, nsLayoutUtils::PaintFrameFlags) nsLayoutUtils.cpp:3428
     mozilla::PresShell::PaintInternal(nsView *, mozilla::PaintInternalFlags) PresShell.cpp:6438
     nsViewManager::ProcessPendingUpdatesPaint(nsIWidget *) nsViewManager.cpp:433
     nsViewManager::ProcessPendingUpdatesForView(nsView *, bool) nsViewManager.cpp:368
     nsViewManager::ProcessPendingUpdates() nsViewManager.cpp:941
     nsRefreshDriver::Tick(mozilla::layers::BaseTransactionId<…>, mozilla::TimeStamp, nsRefreshDriver::IsExtraTick) nsRefreshDriver.cpp:2760
     [Inlined] mozilla::RefreshDriverTimer::TickDriver(nsRefreshDriver *, mozilla::layers::BaseTransactionId<…>, mozilla::TimeStamp) nsRefreshDriver.cpp:373
     mozilla::RefreshDriverTimer::TickRefreshDrivers(mozilla::layers::BaseTransactionId<…>, mozilla::TimeStamp, nsTArray<…> &) nsRefreshDriver.cpp:351
     mozilla::RefreshDriverTimer::Tick(mozilla::layers::BaseTransactionId<…>, mozilla::TimeStamp) nsRefreshDriver.cpp:367
     mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers(mozilla::layers::BaseTransactionId<…>, mozilla::TimeStamp) nsRefreshDriver.cpp:911
     mozilla::VsyncRefreshDriverTimer::TickRefreshDriver(mozilla::layers::BaseTransactionId<…>, mozilla::TimeStamp) nsRefreshDriver.cpp:825
     mozilla::VsyncRefreshDriverTimer::NotifyVsyncOnMainThread(const mozilla::VsyncEvent &) nsRefreshDriver.cpp:746
     mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsyncTimerOnMainThread() nsRefreshDriver.cpp:592
     mozilla::dom::VsyncMainChild::RecvNotify(const mozilla::VsyncEvent &, const float &) VsyncMainChild.cpp:66

And in other cases the page is rendered except for the video, and there is a spinner spinning indefinitely on top of the video. In this case the main thread is stuck at:

     mozilla::layers::ImageContainer::HasCurrentImage() ImageContainer.cpp:386
     mozilla::dom::HTMLMediaElement::UpdateReadyStateInternal() HTMLMediaElement.cpp:5838
     [Inlined] <lambda>::operator()() const StateWatching.h:251
     mozilla::detail::RunnableFunction::Run() nsThreadUtils.h:548
     mozilla::SimpleTaskQueue::DrainTasks() TaskDispatcher.h:44
     [Inlined] nsThread::DrainDirectTasks() nsThread.cpp:1437
     non-virtual thunk to nsThread::DrainDirectTasks() nsThread.cpp:0
     [Inlined] mozilla::AutoTaskDispatcher::TaskGroupRunnable::MaybeDrainDirectTasks() TaskDispatcher.h:243
     mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run() TaskDispatcher.h:226
     mozilla::XPCOMThreadWrapper::Runner::Run() AbstractThread.cpp:208
     mozilla::RunnableTask::Run() TaskController.cpp:555
     mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(const mozilla::detail::BaseAutoLock<…> &) TaskController.cpp:879
     mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(const mozilla::detail::BaseAutoLock<…> &) TaskController.cpp:702
     mozilla::TaskController::ProcessPendingMTTask(bool) TaskController.cpp:491
     [Inlined] $_0::operator()() const TaskController.cpp:218
     mozilla::detail::RunnableFunction::Run() nsThreadUtils.h:548
     nsThread::ProcessNextEvent(bool, bool *) nsThread.cpp:1239

In both cases the ImageBridgeChild thread is stuck at:

     [Inlined] mozilla::layers::ImageContainer::GetImageContainerListener() ImageContainer.h:548
     mozilla::layers::ImageBridgeChild::Connect(mozilla::layers::CompositableClient *, mozilla::layers::ImageContainer *) ImageBridgeChild.cpp:280
     mozilla::layers::CompositableClient::Connect(mozilla::layers::ImageContainer *) CompositableClient.cpp:63
     [Inlined] mozilla::layers::ImageBridgeChild::CreateImageClientNow(mozilla::layers::CompositableType, mozilla::layers::ImageContainer *) ImageBridgeChild.cpp:652
     mozilla::layers::ImageBridgeChild::CreateImageClientSync(mozilla::layers::SynchronousTask *, RefPtr<…> *, mozilla::layers::CompositableType, mozilla::layers::ImageContainer *) ImageBridgeChild.cpp:242
     mozilla::detail::runnable_args_base::Run() runnable_utils.h:41
     nsThread::ProcessNextEvent(bool, bool *) nsThread.cpp:1233

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1827024

:aosmond, since you are the author of the regressor, bug 1827024, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 2

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1831099 - Remove lock requirement for ImageContainer::mNotifyCompositeListener to avoid deadlock. r=jnicol

In bug 1827024 we added a lock to protect mNotifyCompositeListener but
this turned out to be overzealous and unnecessary as we currently only
ever access the member on the ImageBridge thread. We can just assert
instead to ensure we don't regress on this. Removing the lock
requirement solves a deadlock.

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Sounds like this may be an RC respin driver.

Comment 4

[Andrew Osmond [:aosmond] (he/him)]

try for mozilla-central/mozilla-beta: https://treeherder.mozilla.org/jobs?repo=try&revision=dd0d98caccee59024cba953972615f427f22fc80
try for mozilla-esr102: https://treeherder.mozilla.org/jobs?repo=try&revision=1c35c161ec195ba4963866796267c9df9e2a2572

Comment 5

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Rebase for mozilla-esr, v1

Comment 6

[Andrew Osmond [:aosmond] (he/him)]

jnicol confirmed the patch fixes the issue in a local build.

Comment 7

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 9331368 [details]
Bug 1831099 - Remove lock requirement for ImageContainer::mNotifyCompositeListener to avoid deadlock. r=jnicol

Beta/Release Uplift Approval Request

• User impact if declined: App may freeze when GPU process crashes and we try to recover
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): We manually reproduced without the fix, could not reproduce with. It just removes a lock that we didn't have before that turns out to be unnecessary given the constraints of how we use it. I added asserts to ensure this remains true.
• String changes made/needed:
• Is Android affected?: Yes

Comment 8

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 9331390 [details] [diff] [review]
Rebase for mozilla-esr, v1

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Regression from change we uplifted to ESR in bug 1827024
• User impact if declined: App may freeze when GPU process crashes and we try to recover
• Fix Landed on Version: 114
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): We manually reproduced without the fix, could not reproduce with. It just removes a lock that we didn't have before that turns out to be unnecessary given the constraints of how we use it. I added asserts to ensure this remains true.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3b37c0ddd1ea
Remove lock requirement for ImageContainer::mNotifyCompositeListener to avoid deadlock. r=jnicol

Comment 10

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/3b37c0ddd1ea

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:aosmond, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox113 to wontfix.

For more information, please visit BugBot documentation.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9331390 [details] [diff] [review]
Rebase for mozilla-esr, v1

Approved for 102.11esr build2.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr102/rev/77dd215a134a

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9331368 [details]
Bug 1831099 - Remove lock requirement for ImageContainer::mNotifyCompositeListener to avoid deadlock. r=jnicol

Approved for 113.0 RC2.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/ff71ab81d741