Closed Bug 1819742 Opened 2 years ago Closed 2 years ago

MOZ_RELEASE_ASSERT((mType) == (aType)) (unexpected type tag) Crash in [@ mozilla::dom::BodyStreamVariant::AssertSanity]

Categories

(Core :: DOM: Networking, defect, P1)

Unspecified
Android
defect

Tracking

()

RESOLVED FIXED
112 Branch
Tracking Status
firefox-esr102 --- unaffected
firefox110 --- unaffected
firefox111 --- fixed
firefox112 --- fixed

People

(Reporter: cpeterson, Assigned: edenchuang)

References

(Regression)

Details

(Keywords: crash, regression, Whiteboard: [necko-triaged])

Crash Data

Attachments

(1 file)

This MOZ_RELEASE_ASSERT crash looks like a regression in 111.0a1. For some reason, it only affects Android.

Crash report: https://crash-stats.mozilla.org/report/index/b6921d1c-6c82-4d84-8d16-4e8e20230228

MOZ_CRASH Reason: MOZ_RELEASE_ASSERT((mType) == (aType)) (unexpected type tag)

Top 10 frames of crashing thread:

0  libxul.so  mozilla::dom::BodyStreamVariant::AssertSanity const  ipc/ipdl/_ipdlheaders/mozilla/dom/FetchTypes.h
1  libxul.so  mozilla::dom::BodyStreamVariant::get_ChildToParentStream const  ipc/ipdl/_ipdlheaders/mozilla/dom/FetchTypes.h:565
1  libxul.so  mozilla::dom::InternalRequest::InternalRequest  dom/fetch/InternalRequest.cpp:202
2  libxul.so  mozilla::MakeSafeRefPtr<mozilla::dom::InternalRequest, mozilla::dom::IPCInternalRequest const&>  dom/indexedDB/SafeRefPtr.h:379
2  libxul.so  mozilla::dom::FetchEventOpProxyChild::Initialize  dom/serviceworkers/FetchEventOpProxyChild.cpp:70
3  libxul.so  mozilla::dom::RemoteWorkerChild::RecvPFetchEventOpProxyConstructor  dom/workers/remoteworkers/RemoteWorkerChild.cpp:1115
3  libxul.so  mozilla::dom::PRemoteWorkerChild::OnMessageReceived  ipc/ipdl/PRemoteWorkerChild.cpp:455
4  libxul.so  mozilla::ipc::PBackgroundChild::OnMessageReceived  ipc/ipdl/PBackgroundChild.cpp:6306
5  libxul.so  mozilla::ipc::MessageChannel::DispatchAsyncMessage  ipc/glue/MessageChannel.cpp:1800
5  libxul.so  mozilla::ipc::MessageChannel::DispatchMessage  ipc/glue/MessageChannel.cpp:1725

Looks like this was added in bug 1351231.

Flags: needinfo?(echuang)
Regressed by: 1351231

I am not sure, but I guess the root cause is android's processes model is a little bit different from the desktop version.

According to the core stack, this line

2 libxul.so mozilla::dom::FetchEventOpProxyChild::Initialize dom/serviceworkers/FetchEventOpProxyChild.cpp:70

shows that the thread(the worker launcher thread) is supposed on the content process.
And the request's body is transferred from the parent process to the content process, so it supposes to be a TParentToChildStream.

However, the line

1 libxul.so mozilla::dom::BodyStreamVariant::get_ChildToParentStream const ipc/ipdl/_ipdlheaders/mozilla/dom/FetchTypes.h:565

shows that we are trying to get a TChildToParentStream, which is not our expectation.
But it fails with an assertion since it actually is a TParentToChildStream.

So, I guess there is something wrong here with the android platform
https://searchfox.org/mozilla-central/rev/f7edb0b474a1a922f3285107620e802c6e19914d/dom/fetch/InternalRequest.cpp#193,198
The code logic wants to ensure that the correct stream type is used on the correct processes.
When in the parent process, we should never get a TParentToChildStream. And vice-versa.
However, this seems not to be guaranteed on the android platform.

Here is a solution we do not check the relationship between processes and stream type and just directly set up the stream according to its body type.

Assignee: nobody → echuang
Flags: needinfo?(echuang)
Severity: -- → S2
Priority: -- → P1
Whiteboard: [necko-triaged]
Pushed by echuang@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b94def6b6a8b Directly set up stream according to the IPC stream type in InternalRequest. r=dom-worker-reviewers,smaug
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 112 Branch

Eden, this crash affects Fenix 111, which is shipping to Release next week. Should we uplift your fix to a 111.0.1 dot release the week after?

Flags: needinfo?(echuang)

Comment on attachment 9320751 [details]
Bug 1819742 - Directly set up stream according to the IPC stream type in InternalRequest. r=#dom-worker-reviewers

Beta/Release Uplift Approval Request

  • User impact if declined: The user could meet unexpected crashes while visiting websites that use ServiceWorker on the Android platform.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • 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): The patch skips the process type and stream type checking which is not correct for the Android platform process model.
  • String changes made/needed: No
  • Is Android affected?: Yes
Flags: needinfo?(echuang)
Attachment #9320751 - Flags: approval-mozilla-beta?

Comment on attachment 9320751 [details]
Bug 1819742 - Directly set up stream according to the IPC stream type in InternalRequest. r=#dom-worker-reviewers

Approved for 111.0 RC1 and Fenix/Focus 111.0 RC1

Attachment #9320751 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: