Closed Bug 1675820 Opened 2 years ago Closed 1 year ago

Crash in [@ IPCError-browser | RecvCreateBrowsingContext Parent has different group object]

Categories

(Core :: DOM: Navigation, defect, P3)

Unspecified
Windows 10
defect

Tracking

()

RESOLVED FIXED
88 Branch
Fission Milestone M7a
Tracking Status
firefox-esr78 --- unaffected
firefox86 --- disabled
firefox87 --- disabled
firefox88 --- fixed

People

(Reporter: mccr8, Assigned: nika)

References

Details

(Keywords: crash)

Crash Data

Attachments

(7 files, 1 obsolete file)

Crash report: https://crash-stats.mozilla.org/report/index/c630dda1-b3ad-4f36-926d-9cd370201106

Reason: EXCEPTION_BREAKPOINT

Top 9 frames of crashing thread:

0 ntdll.dll ntdll.dll@0x9c7a4 
1 kernelbase.dll VirtualProtect 
2 xul.dll js::jit::Linker::newCode js/src/jit/Linker.cpp:70
3 xul.dll js::jit::BaselineCompiler::compile js/src/jit/BaselineCodeGen.cpp:253
4 xul.dll js::jit::BaselineCompile js/src/jit/BaselineJIT.cpp:250
5 xul.dll js::jit::BaselineCompileFromBaselineInterpreter js/src/jit/BaselineJIT.cpp:446
6  @0x1caa60624c3 
7 xul.dll js::jit::DoNewArrayFallback js/src/jit/BaselineIC.cpp:3922
8  @0x933b1faa57 

Bug 1656854 fixed the big spike of this crash in August, but it is still happening at a lower level.

M7 because the crash volume is not that high.

A subframe is being created after its parent frame or group has already been discarded.

Assignee: nobody → nika
Severity: -- → S2
Fission Milestone: --- → M7
Priority: -- → P3

This seems to still be happening. Some notes from my quick scan during triage: Maybe being caused by a message sent over PContent even though the PBrowser is already dead which the BrowsingContext is embedded in? Should we consider having the message be sent over PBrowser instead?

The previous implementation which used WindowContext was unncessarially
complicated, and can be simplified.

The issues with BrowsingContextGroup identity may be related to using a
destroyed BrowsingContextGroup in some situations when we shouldn't be. By
upgrading the intensity of these assertions, we should be able to catch the
issues more readily.

When these fields were added, it appears I completely forgot to add them to the
cycle collection unlink logic. This somehow didn't cause leaks due to existing
code which would break the cycles between the inner window and the outer window
/ BrowsingContext.

Previously it appears that WindowContext::Discard was never invoked in the
content process which hosts the window in question, which may have lead to the
crashes mentioned in this issue, among other problems.

This allows for the WindowGlobalChild getter in WindowContext to be acquired
more efficiently without performing hashtable lookups, and should generally
simplify things.

The patch also removes the unnecessary XRE_IsContentProcess assertions, and
removes the global hashtable for tracking WindowGlobalChild instances which is
no longer necessary.

This should further reduce the chance that a BrowsingContextGroup is mentioned
in a message which has already ben destroyed by a remote process.

Attachment #9208554 - Attachment is obsolete: true
Attachment #9208555 - Attachment description: Bug 1675820 - Part 2: Upgrade destroyed asserts in BrowsingContextGroup, → Bug 1675820 - Part 1: Upgrade destroyed asserts in BrowsingContextGroup,
Attachment #9208556 - Attachment description: Bug 1675820 - Part 3: Trace inner window mBrowsingContext and mWindowGlobalChild fields, → Bug 1675820 - Part 2: Trace inner window mBrowsingContext and mWindowGlobalChild fields,
Attachment #9208557 - Attachment description: Bug 1675820 - Part 4: Ensure WindowContext is discarded when WindowGlobalChild is destroyed, → Bug 1675820 - Part 3: Ensure WindowContext is discarded when WindowGlobalChild is destroyed,
Attachment #9208558 - Attachment description: Bug 1675820 - Part 5: Track WindowGlobalChild in WindowContext, → Bug 1675820 - Part 4: Track WindowGlobalChild in WindowContext,
Attachment #9208559 - Attachment description: Bug 1675820 - Part 6: Keep BCGs alive while waiting for WindowContext discards to be acked, → Bug 1675820 - Part 5: Keep BCGs alive while waiting for WindowContext discards to be acked,

Previously, the DocGroup type was not cycle-collected, as it needed to have
references from other threads for Quantum DOM. Nowadays the only off-main-thread
use of DocGroup is for dispatching runnables to the main thread which should be
tracked using a performance counter for about:performance. This means we can
remove the DocGroup references from these dispatching callsites, only storing
the Performance Counter we're interested in, and simplify make DocGroup be
cycle-collected itself.

This fixes a leak caused by adding the WindowGlobalChild getter to
WindowContext, by allowing cycles between the document and its BrowsingContext
to be broken by the cycle-collector.

Fission Milestone: M7 → M7a
Pushed by nlayzell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d5fd9d265e2c
Part 1: Upgrade destroyed asserts in BrowsingContextGroup, r=kmag
https://hg.mozilla.org/integration/autoland/rev/83789537d242
Part 2: Trace inner window mBrowsingContext and mWindowGlobalChild fields, r=smaug
https://hg.mozilla.org/integration/autoland/rev/c49f76a036ac
Part 3: Ensure WindowContext is discarded when WindowGlobalChild is destroyed, r=kmag
https://hg.mozilla.org/integration/autoland/rev/1a9778d2215c
Part 4: Track WindowGlobalChild in WindowContext, r=kmag
https://hg.mozilla.org/integration/autoland/rev/6417dd804869
Part 5: Keep BCGs alive while waiting for WindowContext discards to be acked, r=kmag
https://hg.mozilla.org/integration/autoland/rev/e6d84dd114ed
Part 6: Make DocGroup cycle-collected, r=farre,smaug
https://hg.mozilla.org/integration/autoland/rev/f2ed6497335f
Part 7: Fix marionette BrowsingContext replace detection logic, r=whimboo,marionette-reviewers,farre
Regressions: 1699651
Regressions: 1699626
Regressions: 1699628
See Also: → 1535913
You need to log in before you can comment on or make changes to this bug.