Crash in nsDOMMutationRecord::Release

VERIFIED FIXED in Firefox 65

Status

()

defect
--
critical
VERIFIED FIXED
4 months ago
a month ago

People

(Reporter: calixte, Assigned: smaug)

Tracking

(Blocks 1 bug, {crash, regression})

Trunk
mozilla66
Unspecified
Windows 10
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox64 wontfix, firefox65 verified, firefox66 verified)

Details

(crash signature)

Attachments

(2 attachments)

(Reporter)

Description

4 months ago
This bug was filed from the Socorro interface and is
report bp-84b0c900-c4f2-47e6-851b-8c2ae0190102.
=============================================================

Top 1 frames of crashing thread:

0 xul.dll nsDOMMutationRecord::Release dom/base/nsDOMMutationObserver.cpp:61

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

There are 2 crashes (from 1 installation) in nightly 66 with buildid 20190101212914. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1514802.

[1] https://hg.mozilla.org/mozilla-central/rev?node=13191d88873d
Flags: needinfo?(bugs)
Both of those are from me. The crash is reproducible but probably only happens in extreme situations like a big MedSpace + LastPass. It was indeed caused by the fix to bug 1514802 (I'll leave the needinfo in case smaug wants to use this bug to land a follow-up).
(Assignee)

Comment 2

4 months ago
yup, I'm fixing the issue in a bit different way.
MedSpace must be causing insane amount of dom mutations :/
Assignee: nobody → bugs
Flags: needinfo?(bugs)
(Assignee)

Comment 3

4 months ago
Attachment #9033949 - Flags: review?(continuation)
(In reply to Olli Pettay [:smaug] from comment #2)
> MedSpace must be causing insane amount of dom mutations :/

That's probably true.. Unfortunately I think it's not something we fully control as Notes generates a lot of code from the design elements. But that's not something I know for sure yet, and profiling this old code is certainly part of my job description :)
Attachment #9033949 - Flags: review?(continuation) → review+

Comment 5

4 months ago
Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/64b2e707cdbd
explicitly break down pending mutation record list, r=mccr8

Comment 6

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/64b2e707cdbd
Status: NEW → RESOLVED
Last Resolved: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
I can confirm that both the hangs and the crashes appear to be gone in today's Nightly!
(Assignee)

Comment 8

4 months ago
[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: None

User impact if declined: Long CC passes like in bug 1514802

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: bug  1514802 has a testcase. If it hangs, the issue is not fixed

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Just explicitly clearing a list of objects which is anyhow going to be deleted soon.

String changes made/needed: NA
Attachment #9034572 - Flags: approval-mozilla-beta?
Flags: qe-verify+
Comment on attachment 9034572 [details] [diff] [review]
mo_cc_hang_beta.diff

[Triage Comment]
Fixes some pretty bad hangs as reported in bug 1514802. Approved for 65.0b9.
Attachment #9034572 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

I've tried to reproduce this bug on Windows 10 x64 (using an affected Nightly build from 2018-12-17), but I wasn't able to reproduce the crash nor the hangs with the test case mentioned in bug 1514802 comment 23. Also, I didn't observe any hangs on https://www.medspace.nl/ on my machine.

Emanuel, I see that you've already verified this fix on Nightly 66, could you please help us to confirm this fix on Beta 65.0b9 [1] as well; just to make sure that everything works as expected in that build, too. Thank you!

[1] https://archive.mozilla.org/pub/firefox/candidates/65.0b9-candidates/build1/win64/en-US/

Flags: needinfo?(emanuel.hoogeveen)

Yes, this is fixed in Beta 65.0b9 as well. I tested the large MedSpace that caused the crashes in this bug and saw no crashes or hangs.

Flags: needinfo?(emanuel.hoogeveen)
Status: RESOLVED → VERIFIED

Thank you for verifying this, Emanuel!

Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.