1761208 - startup Crash in [@ mozilla::net::LoadInfo::SetReservedClientInfo] on Amazon

Status: NEW

(Core :: DOM: Service Workers, defect, P3)

Description

[Kershaw Chang [:kershaw]]

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

As per this crash report, the crash reason now is MOZ_DIAGNOSTIC_ASSERT(false) (mReservedClientInfo already set).

Top 10 frames of crashing thread:

0 xul.dll mozilla::net::LoadInfo::SetReservedClientInfo netwerk/base/LoadInfo.cpp:1771
1 xul.dll mozilla::dom::`anonymous namespace'::ClientChannelHelperParent::CreateClientForPrincipal dom/clients/manager/ClientChannelHelper.cpp:247
2 xul.dll mozilla::dom::`anonymous namespace'::ClientChannelHelperParent::CreateClient dom/clients/manager/ClientChannelHelper.cpp:213
3 xul.dll mozilla::dom::`anonymous namespace'::ClientChannelHelper::AsyncOnChannelRedirect dom/clients/manager/ClientChannelHelper.cpp:143
4 xul.dll mozilla::net::nsAsyncRedirectVerifyHelper::DelegateOnChannelRedirect netwerk/base/nsAsyncRedirectVerifyHelper.cpp:153
5 xul.dll mozilla::net::nsAsyncRedirectVerifyHelper::Run netwerk/base/nsAsyncRedirectVerifyHelper.cpp:259
6 xul.dll mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal xpcom/threads/TaskController.cpp:805
7 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1152
8 xul.dll mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:85
9 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:324

Comment 1

[Jens Stutte [:jstutte]]

Could this be related to bug 1758125 ?

Comment 2

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

(I don't know very much about workers.) Maybe in an abstract sense? Bug 1758125 is really just an observation that LoadInfo for Workers has several different paths by which it can be initialized; and that's prone to bugs. This crash seems to be related to dom/clients and I'm not even sure what that is....

Comment 3

[Jens Stutte [:jstutte]]

Hi Andrew, just a wild guess, might be far off.

Comment 4

[Andrew Sutherland [:asuth] (he/him)]

I think this is a continuation of https://bugzilla.mozilla.org/show_bug.cgi?id=1759506#c26 and so Eden is the right NI.

Comment 5

[Eden Chuang[:edenchuang]]

As commented on https://bugzilla.mozilla.org/show_bug.cgi?id=1759506#c26, what we saw is the redirect loadinfo.mReservedClientInfo had already been set before ClientChannelHelper creates a new one for it.

nsILoadInfo::SetReservedClientInfo is called in DocumentLoadListener and HttpChannelParent

According to the comment here https://searchfox.org/mozilla-central/source/netwerk/protocol/http/HttpChannelParent.cpp#1704. It seems that we have wrong assumption that "child ClientChannelHelper will handle the client info propagation." In fact, ClientInfo propagation is handled in the parent. So here might be the root cause that why loadInfo.mReservedClientInfo is not Nothing().

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Seeing a bit of a recent spike for this on Nightly, still mostly with Amazon sites. Any chance you could take another look at this, Eden?

Comment 7

[Eden Chuang[:edenchuang]]

I am still trying to figure out the reason why the new channel's LoadInfo's reservedClientInfo has been set before getting into ClientChannelHelper::AsyncOnChannelRedirect(). Unfortunately, the crash stacks do not provide useful information about it.

After looking at source codes about setting nsILoadInfo reservedClientInfo, I doubt that this is caused by loading a document in the parent process, which is the case when using ParentProcessDocumentChannel to load a document.
When ParentProcessDocumentChannel is used, both ClientChannalHelperChid and ClientChannelHeplerParent are registered to handle the ClientInfo propagation for channel redirection. However, we must ensure the execution sequence between them is correct.

I am checking that if it is a possible case that ClientChannelHelperChild::AsyncOnChannelRedirect() could be executed before ClientChannelHelper::AsyncOnChannelRedirect(). If there is a case, we could meet this assertion.

Comment 8

[Donal Meehan [:dmeehan]]

:edenchuang any updates on Comment 7?
Fx121 is now in beta and there is a small spike in crashes

Comment 9

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

The bug is linked to a topcrash signature, which matches the following criterion:

• Top 20 desktop browser crashes on beta

:jjalkanen, could you consider increasing the severity of this top-crash bug?

For more information, please visit BugBot documentation.

Comment 10

[Martin]

Hello, suddenly I started to see similar (or the same ?) issue (e.g. Crash ID: 0364773f-988d-4282-b83c-87b490231211). What helped to me was to disable "Use recommended performance settings" in Performance setting. When I re-enabled this, Firefox started to crash again (it crashed also in troubleshot mode without enabled extensions).

Comment 11

[Martin]

(In reply to Martin from comment #10)

Hello, suddenly I started to see similar (or the same ?) issue (e.g. Crash ID: 0364773f-988d-4282-b83c-87b490231211). What helped to me was to disable "Use recommended performance settings" in Performance setting. When I re-enabled this, Firefox started to crash again (it crashed also in troubleshot mode without enabled extensions).

Unfortunately performance settings is not probably 100% workaround. While it looks it helped yesterday for some url (www.seznam.cz) (and today I can still duplicate it), some others (amazon.com) are still crashing Firefox (even with new profile and in troubleshoot mode).

Comment 12

[Yannis Juglaret [:yannis]]

(In reply to Eden Chuang[:edenchuang] from comment #7)

I am still trying to figure out the reason why the new channel's LoadInfo's reservedClientInfo has been set before getting into ClientChannelHelper::AsyncOnChannelRedirect(). Unfortunately, the crash stacks do not provide useful information about it.

I tried to add information about this through bug 1868304. According to the first crash report that we've received which embeds this information, the reservedClientInfo was set here with the following call stack:

00 xul!mozilla::dom::`anonymous namespace'::ClientChannelHelperParent::CreateClientForPrincipal+0x49 [/builds/worker/checkouts/gecko/dom/clients/manager/ClientChannelHelper.cpp @ 251]
01 xul!mozilla::net::DocumentLoadListener::Open+0xddc [/builds/worker/checkouts/gecko/netwerk/ipc/DocumentLoadListener.cpp @ 806]
02 xul!mozilla::net::DocumentLoadListener::OpenDocument+0x1c4 [/builds/worker/checkouts/gecko/netwerk/ipc/DocumentLoadListener.cpp @ 979]
03 xul!mozilla::net::NeckoParent::RecvPDocumentChannelConstructor+0x20e [/builds/worker/checkouts/gecko/netwerk/ipc/NeckoParent.cpp @ 281]
04 xul!mozilla::net::PNeckoParent::OnMessageReceived+0x1c25 [/builds/worker/workspace/obj-build/ipc/ipdl/PNeckoParent.cpp @ 1985]
05 xul!mozilla::dom::PContentParent::OnMessageReceived+0x171 [/builds/worker/workspace/obj-build/ipc/ipdl/PContentParent.cpp @ 6828]
06 xul!mozilla::ipc::MessageChannel::MessageTask::Run+0x776 [/builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp @ 1632]
07 xul!mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal+0x1354 [/builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp @ 876]
08 xul!NS_ProcessNextEvent+0xbd8 [/builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp @ 480]
09 xul!mozilla::ipc::MessagePump::Run+0x2b0 [/builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp @ 86]
0a xul!MessageLoop::RunHandler+0x2f [/builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc @ 364]
0b xul!nsBaseAppShell::Run+0xa2 [/builds/worker/checkouts/gecko/widget/nsBaseAppShell.cpp @ 148]
0c xul!nsAppShell::Run+0x32 [/builds/worker/checkouts/gecko/widget/windows/nsAppShell.cpp @ 824]
0d xul!nsAppStartup::Run+0x41 [/builds/worker/checkouts/gecko/toolkit/components/startup/nsAppStartup.cpp @ 297]
0e xul!XREMain::XRE_mainRun+0xcea [/builds/worker/checkouts/gecko/toolkit/xre/nsAppRunner.cpp @ 5709]
0f xul!XREMain::XRE_main+0x302 [/builds/worker/checkouts/gecko/toolkit/xre/nsAppRunner.cpp @ 5918]
10 xul!XRE_main+0x85 [/builds/worker/checkouts/gecko/toolkit/xre/nsAppRunner.cpp @ 5974]
11 firefox!wmain+0x2406 [/builds/worker/checkouts/gecko/toolkit/xre/nsWindowsWMain.cpp @ 151]
12 firefox!__scrt_common_main_seh+0x10c [D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl @ 288]
13 kernel32!BaseThreadInitThunk+0x14
14 ntdll!RtlUserThreadStart+0x21

Does this help?

Comment 13

[Yannis Juglaret [:yannis]]

The stack trace for emplace is the same in all 7 crashes we have received, I'll remove the instrumentation now that we know this.

Comment 14

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

Based on the topcrash criteria, the crash signatures linked to this bug are not in the topcrash signatures anymore.

For more information, please visit BugBot documentation.

Comment 15

[Eden Chuang[:edenchuang]]

Attachment: Bug 1761208 - Ensure pre-created reservedClient has the same type and principal. r=#dom-worker-reviewers