Closed Bug 1470114 Opened Last year Closed Last year

Crash in mozilla::Maybe<T>::emplace<T>

Categories

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

62 Branch
All
Windows
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox60 --- unaffected
firefox61 --- unaffected
firefox62 --- fixed

People

(Reporter: philipp, Assigned: bkelly)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is
report bp-2fe8ef57-8d1f-43d1-9c85-1c3ce0180618.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll mozilla::Maybe<mozilla::dom::ClientInfo>::emplace<mozilla::dom::ClientInfo const&> mfbt/Maybe.h:598
1 xul.dll mozilla::net::LoadInfo::SetReservedClientInfo netwerk/base/LoadInfo.cpp:1326
2 xul.dll mozilla::ipc::MergeChildLoadInfoForwarder ipc/glue/BackgroundUtils.cpp:718
3 xul.dll mozilla::net::HttpChannelParent::RecvRedirect2Verify netwerk/protocol/http/HttpChannelParent.cpp:950
4 xul.dll mozilla::net::PHttpChannelParent::OnMessageReceived ipc/ipdl/PHttpChannelParent.cpp:750
5 xul.dll mozilla::dom::PContentParent::OnMessageReceived ipc/ipdl/PContentParent.cpp:3501
6 xul.dll mozilla::ipc::MessageChannel::DispatchAsyncMessage ipc/glue/MessageChannel.cpp:2134
7 xul.dll mozilla::ipc::MessageChannel::DispatchMessageW ipc/glue/MessageChannel.cpp:2064
8 xul.dll mozilla::ipc::MessageChannel::RunMessage ipc/glue/MessageChannel.cpp:1910
9 xul.dll mozilla::ipc::MessageChannel::MessageTask::Run ipc/glue/MessageChannel.cpp:1943

=============================================================

crash reports with this stack are regressing in firefox 62 which is probably caused by bug 1441932, as the first occurrence of the crash was in 62.0a1 build 20180605141053.
all the reports till now come from windows installations and have MOZ_RELEASE_ASSERT(!mIsSome).
Assignee: nobody → bkelly
Status: NEW → ASSIGNED
Flags: needinfo?(bkelly)
Comment on attachment 8986916 [details] [diff] [review]
Make parent-side of redirect override the reserved ClientInfo to handle some corner cases. r=valentin

So this crash is occurring when the child-process sends a new/different reserved ClientInfo to the parent for a redirected channel.  A new reserved ClientInfo is expected for cross-origin redirects and is implemented by this code in the child-process:

https://searchfox.org/mozilla-central/rev/93d2b9860b3d341258c7c5dcd4e278dea544432b/dom/clients/manager/ClientChannelHelper.cpp#107-127

Now, when the child-process approves the redirect (with the new reserved client), we send the reserved ClientInfo back over to the parent process to put in the new channel's LoadInfo there.  If we try to emplace the ClientInfo into the LoadInfo's Maybe<> value we get this crash.

But normally there should not be anything in the LoadInfo's mReservedClientInfo Maybe<> value.  The copy constructor for the LoadInfo does not automatically copy across the reserved ClientInfo value.  It needs special handling via the ClientChannelHelper, etc.  See:

https://searchfox.org/mozilla-central/rev/d0a41d2e7770fc00df7844d5f840067cc35ba26f/netwerk/base/LoadInfo.cpp#360

So there is a bit of a mystery as to why the new channel already has a reserved ClientInfo in the parent.  My best guess is that there are some cases where one of the following happens on redirect:

1. The old and new channel for the redirect are actually the same channel.  So they have the same LoadInfo.
2. The old and new channel are different, but share the same LoadInfo.

I believe I have seen something like these cases happen before, but never understood exactly what triggered it.  Perhaps something in web extensions, proxies, or another weird corner case.  Unfortunately I don't have time to track down why we are hitting this case.

Instead, in this patch I propose we simply make the redirect authoritatively override the reserved ClientInfo.  I add a new method with documentation that it should only be used for this purpose.  It clears any previous value for the LoadInfo::mReservedClientInfo and emplaces the new value.
Attachment #8986916 - Flags: review?(valentin.gosu)
Attachment #8986916 - Flags: review?(valentin.gosu) → review+
Pushed by bkelly@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/20397d7932b2
Make parent-side of redirect override the reserved ClientInfo to handle some corner cases. r=valentin
https://hg.mozilla.org/mozilla-central/rev/20397d7932b2
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Duplicate of this bug: 1469894
You need to log in before you can comment on or make changes to this bug.