1826875 - Crash in [@ mozilla::ipc::WeakActorLifecycleProxy::Get]

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Sean Feng [:sefeng211]]

Crash report: https://crash-stats.mozilla.org/report/index/8331f5f1-f0d0-49cf-a642-142940230227

MOZ_CRASH Reason: MOZ_DIAGNOSTIC_ASSERT(mActorEventTarget->IsOnCurrentThread())

Top 10 frames of crashing thread:

0  xul.dll  mozilla::ipc::WeakActorLifecycleProxy::Get const  ipc/glue/ProtocolUtils.cpp:272
0  xul.dll  mozilla::ipc::IPDLResolverInner::ResolveOrReject  ipc/glue/ProtocolUtils.cpp:801
1  xul.dll  std::_Func_impl_no_alloc<`lambda at /builds/worker/workspace/obj-build/ipc/ipdl/PBackgroundParent.cpp:6289:68', void, mozilla::Tuple<const nsresult&, mozilla::ipc::Endpoint<mozilla::PRemoteDecoderManagerChild>&&> >::_Do_call  ipc/glue/ProtocolUtils.h:684
2  xul.dll  std::_Func_class<void, mozilla::Tuple<const nsresult&, mozilla::ipc::Endpoint<mozilla::PRemoteDecoderManagerChild>&&> >::_Empty const  /builds/worker/fetches/vs/vc/tools/msvc/14.16.27023/include/functional:1283
2  xul.dll  std::_Func_class<void, mozilla::Tuple<const nsresult&, mozilla::ipc::Endpoint<mozilla::PRemoteDecoderManagerChild>&&> >::operator const  /builds/worker/fetches/vs/vc/tools/msvc/14.16.27023/include/functional:15732480
2  xul.dll  mozilla::ipc::BackgroundParentImpl::RecvEnsureUtilityProcessAndCreateBridge::<lambda_2>::operator const  ipc/glue/BackgroundParentImpl.cpp:1396
2  xul.dll  mozilla::detail::RunnableFunction<`lambda at /builds/worker/checkouts/gecko/ipc/glue/BackgroundParentImpl.cpp:1390:7'>::Run  xpcom/threads/nsThreadUtils.h:547
3  xul.dll  mozilla::RunnableTask::Run  xpcom/threads/TaskController.cpp:538
3  xul.dll  mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal  xpcom/threads/TaskController.cpp:851
4  xul.dll  mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal  xpcom/threads/TaskController.cpp:683

I checked a few crash reports, the callstack always start with mozilla::ShutdownXPCOM, Looks like it's trying to ensure the utility process is there after the shutdown process has started?

Comment 1

[Andrew McCreight [:mccr8]]

This sounds like a thread safety issue, so I'm going to hide it. However, if it is always during ShutdownXPCOM it probably isn't that bad.

Comment 2

[Andrew McCreight [:mccr8]]

I looked at all of the crashes, and they all have BackgroundParentImpl::RecvEnsureUtilityProcessAndCreateBridge in the stack. Looks like we're in the !upm case. Maybe the dispatch is failing because we're so late in shutdown? Hopefully Alexandre can take a look at some point.

Comment 3

[:gerard-majax]

BackgroundParentImpl::RecvEnsureUtilityProcessAndCreateBridge is ran when something request some media decoding according to https://searchfox.org/mozilla-central/rev/c244b16815d1fc827d141472b9faac5610f250e7/dom/media/ipc/RemoteDecoderManagerChild.cpp#580. Now from the stack, I am not sure exactly what is at fault here. Is it aResolver that causes the crash?

Comment 4

[:gerard-majax]

Also I am not sure I follow the code clearly. Here we verify already the same thing at https://hg.mozilla.org/releases/mozilla-beta/file/18841a00ebe615b1583012945f05d9c8ee5678e9/ipc/glue/ProtocolUtils.cpp#l783

I assume we then hit Get() here https://hg.mozilla.org/releases/mozilla-beta/file/18841a00ebe615b1583012945f05d9c8ee5678e9/ipc/glue/ProtocolUtils.cpp#l788 and this is within this code we hit the assert ? https://hg.mozilla.org/releases/mozilla-beta/file/18841a00ebe615b1583012945f05d9c8ee5678e9/ipc/glue/ProtocolUtils.cpp#l272

Comment 5

[Jens Stutte [:jstutte]]

(In reply to Alexandre LISSY :gerard-majax from comment #4)

Also I am not sure I follow the code clearly. Here we verify already the same thing at https://hg.mozilla.org/releases/mozilla-beta/file/18841a00ebe615b1583012945f05d9c8ee5678e9/ipc/glue/ProtocolUtils.cpp#l783

This is just a plain assertion, not a diagnostic assertion, so it would trigger only in debug builds. Apparently this condition is rare enough to not happen in debug on our CI (at least I cannot find a bug for it).

I assume we then hit Get() here https://hg.mozilla.org/releases/mozilla-beta/file/18841a00ebe615b1583012945f05d9c8ee5678e9/ipc/glue/ProtocolUtils.cpp#l788 and this is within this code we hit the assert ? https://hg.mozilla.org/releases/mozilla-beta/file/18841a00ebe615b1583012945f05d9c8ee5678e9/ipc/glue/ProtocolUtils.cpp#l272

Yeah, and IsOnCurrentThread might pass to IsOnCurrentThreadInfallible which has quite some overrides. And just to make one example: it may return false for shutdown reasons. I am not sure, which event target we should see here, though, but it might have a bad or misleading override?

Comment 6

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

The severity field is not set for this bug.
:jld, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 7

[Nika Layzell [:nika] (ni? for response)]

This is a bug in the code being linked. In an error case, we're invoking the async resolver from the main thread here: https://searchfox.org/mozilla-central/rev/0ffaecaa075887ab07bf4c607c61ea2faa81b172/ipc/glue/BackgroundParentImpl.cpp#1392-1393. It is only safe to invoke IPDL resolvers from the owning thread, so this needs to be done within a dispatch back to the PBackground thread, like how the success case is handled.

Leaving a ni? for :gerard-majax who wrote the code.

Comment 8

[:gerard-majax]

Oops. Clearly I missed than when establishing things from the start ...

Comment 9

[:gerard-majax]

Attachment: Bug 1826875 - Dispatch using correct thread on UtilityProcessManager missing r?#ipc-reviewers!

Comment 10

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

Dispatch using correct thread on UtilityProcessManager missing r=ipc-reviewers,nika
https://hg.mozilla.org/integration/autoland/rev/0fbb64ccaff53c7d219ee2706c4dfb599e70d27e
https://hg.mozilla.org/mozilla-central/rev/0fbb64ccaff5

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for ESR approval when you get a chance. It grafts cleanly as-landed.

Comment 12

[:gerard-majax]

Comment on attachment 9330547 [details]
Bug 1826875 - Dispatch using correct thread on UtilityProcessManager missing r?#ipc-reviewers!

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Simple change to fix the security issue
• User impact if declined: Risk of exploitation during utility process shutdown mostly
• Fix Landed on Version: 114
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Limited scope

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9330547 [details]
Bug 1826875 - Dispatch using correct thread on UtilityProcessManager missing r?#ipc-reviewers!

Approved for 102.12esr.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr102/rev/e7f7b3381b7c