1909374 - Crash in [@ mozilla::Maybe<T>::operator* | mozilla::dom::ContentProcessManager::UnregisterRemoteFrame]

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, defect)

Description

[Andrew McCreight [:mccr8]]

Crash report: https://crash-stats.mozilla.org/report/index/03409c37-3eb4-4853-9e6a-131840240722

MOZ_CRASH Reason: MOZ_RELEASE_ASSERT(isSome())

Top 10 frames:

0  libxul.so  mozilla::Maybe<mozilla::dom::BrowserParent*>::operator*()&  mfbt/Maybe.h:989
0  libxul.so  mozilla::dom::ContentProcessManager::UnregisterRemoteFrame(mozilla::dom::IdTy...  dom/ipc/ContentProcessManager.cpp:91
1  libxul.so  mozilla::dom::BrowserParent::ActorDestroy(mozilla::ipc::IProtocol::ActorDestr...  dom/ipc/BrowserParent.cpp:795
2  libxul.so  mozilla::ipc::IProtocol::ActorDisconnected(mozilla::ipc::IProtocol::ActorDest...  ipc/glue/ProtocolUtils.cpp:623
2  libxul.so  mozilla::ipc::IProtocol::ActorDisconnected(mozilla::ipc::IProtocol::ActorDest...  ipc/glue/ProtocolUtils.cpp:650
3  libxul.so  mozilla::ipc::IProtocol::SetManagerAndRegister(mozilla::ipc::IRefCountedProto...  ipc/glue/ProtocolUtils.cpp:512
4  libxul.so  mozilla::dom::PContentParent::OpenPBrowserEndpoint(mozilla::dom::PBrowserPare...  ipc/ipdl/PContentParent.cpp:594
5  libxul.so  mozilla::dom::ContentParent::CreateBrowser(mozilla::dom::TabContext const&, m...  dom/ipc/ContentParent.cpp:1531
6  libxul.so  nsFrameLoader::TryRemoteBrowserInternal()  dom/base/nsFrameLoader.cpp:2696
6  libxul.so  nsFrameLoader::TryRemoteBrowser()  dom/base/nsFrameLoader.cpp:2761

Crash is on this line: (*childBp)->GetBrowsingContext()->Group()->RemoveKeepAlive();

So I guess childBp is Nothing. In non-release builds, we'd hit a diagnostic assert. Bug 1724083 was filed about fixing this, but it looks like we're still seeing a decent volume in in 128.0.

Comment 1

[Jens Stutte [:jstutte]]

In non-release builds, we'd hit a diagnostic assert.

FWIW, we also hit that diagnostic assertion, it seems: https://crash-stats.mozilla.org/report/index/8c5516c5-d30e-4db6-8d83-a31460240723.

Comment 2

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

Attachment: Bug 1909374 - Register BrowserParent with CPM before calling Bind, r=smaug!

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c8430d0835f8
Register BrowserParent with CPM before calling Bind, r=smaug

Comment 4

[Cristina Horotan [:chorotan]]

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

Comment 5

[Ryan VanderMeulen (PTO, back July 27)]

Should we uplift this to ESR128? It grafts cleanly.

Comment 6

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

Comment on attachment 9414623 [details]
Bug 1909374 - Register BrowserParent with CPM before calling Bind, r=smaug!

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Intermittent crashes when process shutdown and browser creation races in an unfortunate way
• User impact if declined: Intermittent crashes when process shutdown and browser creation races in an unfortunate way
• Fix Landed on Version: 130
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Landed 27 days ago without issues. Small straightforward reordering of operations during process creation to avoid possible failure case.

Comment 7

[Ryan VanderMeulen (PTO, back July 27)]

Comment on attachment 9414623 [details]
Bug 1909374 - Register BrowserParent with CPM before calling Bind, r=smaug!

Approved for 128.2esr.

Comment 8

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/2798da164f0e