Closed Bug 1563542 Opened 1 year ago Closed 1 year ago

Crash in [@ mozilla::dom::ContentParent::RecvAttachBrowsingContext]

Categories

(Core :: DOM: Core & HTML, defect, P1, critical)

69 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla70
Fission Milestone M4
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox67 --- unaffected
firefox68 --- unaffected
firefox69 blocking verified
firefox70 + verified

People

(Reporter: philipp, Assigned: Nika)

References

(Regression)

Details

(Keywords: crash, rca-needed, regression)

Crash Data

Attachments

(1 file)

This bug is for crash report bp-bd690590-ec87-41d2-90c0-f9a610190704.

Top 10 frames of crashing thread:

0 xul.dll mozilla::dom::ContentParent::RecvAttachBrowsingContext dom/ipc/ContentParent.cpp:5755
1 xul.dll mozilla::dom::PContentParent::OnMessageReceived ipc/ipdl/PContentParent.cpp:10538
2 xul.dll mozilla::ipc::MessageChannel::DispatchMessage ipc/glue/MessageChannel.cpp:2082
3 xul.dll mozilla::ipc::MessageChannel::MessageTask::Run ipc/glue/MessageChannel.cpp:1970
4 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1225
5 xul.dll NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:486
6 xul.dll mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:110
7 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:308
8 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:290
9 xul.dll nsBaseAppShell::Run widget/nsBaseAppShell.cpp:137

this browser crash signature with MOZ_RELEASE_ASSERT(parent) (Parent doesn't exist in parent process) is spiking up since in nightly again since 69.0a1 build 20190703094734.
the crashes happen cross platform and account for 30% of browser crashes during the past 24h...

these were the changes going into the first affected nightly build:

Flags: needinfo?(afarre)
Priority: -- → P2

[Tracking Requested - why for this release]: 69.0b2 has a massive number of crashes - almost 3000. Andreas is on PTO until 7-14 - Can we find someone else to help with this? If Bug 1561899 regressed it can we back it out?

Flags: needinfo?(htsai)

Nika since you reviewed the bug in Comment 1 can you please take a look? Thanks.

Flags: needinfo?(nika)

This blocks 69.0b3 gtb with a crash volume that high. Let's backout bug 1561899 from Beta after today's merges are complete.

Flags: needinfo?(aryx.bugmail)
Priority: P2 → P1
Flags: needinfo?(htsai) → needinfo?(nkochar)

Bug 1561899 is a likely cause of the increased crashes so yes, we should back out the patch.

Flags: needinfo?(nkochar)

Nika is looking into this during farre's PTO.

I was able to reproduce the crash on my 10.15 machine by loading https://www.tech-recipes.com/rx/3104/os-x-show-hidden-files-and-folders-in-mac-os-x-finder/. If I wait long enough it seems the browser just crashes on its own - https://crash-stats.mozilla.org/report/index/d8dd2600-ff4e-4a1a-8881-ffb0b0190708.

Hey, I think I found the issue, patch incoming which should be easier to land than a back-out (due to other patches potentially building on top of mIsDetached).

In the mIsDetached bug, the code was changed to not set mClosed during Detach, and only set mIsDetached. This looks like a mistake because a bunch of places are only reading mClosed. Specifically when creating a BrowsingContext for an iframe, we check GetClosed() to see whether to skip creating.

This was changed to be no longer correct, as mClosed was not set during Detach anymore.

(In reply to Neha Kochar [:neha] from comment #4)

This crash signature is already being tracked at Bug 1560106. Bug 1561899 is a likely cause of the increased crashes so yes, we should back out the patch.

That signature is tracking Detach, while this is tracking Attach. This crash also occurs outside of fission-enabled.

Assignee: nobody → nika
Flags: needinfo?(nika)
Regressed by: 1561899

In the bug which introduced mIsDiscarded, the code was changed to not set
mClosed during Detach, and only set mIsDiscarded. This was a mistake because a
bunch of places are only reading mClosed. Specifically when creating a
BrowsingContext for an iframe, we check GetClosed() to see whether to skip
creating it. Not doing this check can lead to assertions like the one in this
bug.

This patch changes the behaviour to continue setting mClosed, and also updates
the relevant GetClosed() checks to correctly check IsDiscarded()

Flags: needinfo?(afarre)
Pushed by nlayzell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6c5ad42069d0
Correctly align usage of mIsDiscarded and mClosed for BrowsingContext, r=peterv

Comment on attachment 9076557 [details]
Bug 1563542 - Correctly align usage of mIsDiscarded and mClosed for BrowsingContext,

Beta/Release Uplift Approval Request

  • User impact if declined: Crashes
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • 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): Small change to restore behaviour.
  • String changes made/needed: none
Attachment #9076557 - Flags: approval-mozilla-beta?

Comment on attachment 9076557 [details]
Bug 1563542 - Correctly align usage of mIsDiscarded and mClosed for BrowsingContext,

Hopefully fixes a topcrash blocking 69. Approved for 69.0b3.

Attachment #9076557 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
Duplicate of this bug: 1563658
Crash Signature: [@ mozilla::dom::ContentParent::RecvAttachBrowsingContext] → [@ mozilla::dom::ContentParent::RecvAttachBrowsingContext] [@ mozilla::dom::ContentParent::RecvAttachBrowsingContext(mozilla::dom::BrowsingContext::IPCInitializer &&)]

Crash is gone \m/

Status: RESOLVED → VERIFIED
Crash Signature: [@ mozilla::dom::ContentParent::RecvAttachBrowsingContext] [@ mozilla::dom::ContentParent::RecvAttachBrowsingContext(mozilla::dom::BrowsingContext::IPCInitializer &&)] → [@ mozilla::dom::ContentParent::RecvAttachBrowsingContext] [@ mozilla::dom::ContentParent::RecvAttachBrowsingContext(mozilla::dom::BrowsingContext::IPCInitializer &&)]
Flags: needinfo?(aryx.bugmail)

Retroactively moving fixed bugs whose summaries mention "Fission" (or other Fission-related keywords) but are not assigned to a Fission Milestone to an appropriate Fission Milestone.

This will generate a lot of bugmail, so you can filter your bugmail for the following UUID and delete them en masse:

0ee3c76a-bc79-4eb2-8d12-05dc0b68e732

Fission Milestone: --- → M4

This bug has been identified as part of a pilot on determining root causes of blocking and dot release drivers.

It needs a root-cause set for it. Please see the list at https://docs.google.com/document/d/1FFEGsmoU8T0N8R9kk-MXWptOPtXXXRRIe4vQo3_HgMw/.

Add the root cause as a whiteboard tag in the form [rca - <cause> ] and remove the rca-needed keyword.

If you have questions, please contact :tmaity.

Keywords: rca-needed
You need to log in before you can comment on or make changes to this bug.