Closed Bug 1422634 Opened 2 years ago Closed 2 years ago

Crash in mozilla::ServoStyleSet::SetStylistStyleSheetsDirty

Categories

(Core :: CSS Parsing and Computation, defect, critical)

Unspecified
Windows 10
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla59
Tracking Status
firefox-esr52 --- unaffected
firefox57 --- unaffected
firefox58 --- unaffected
firefox59 --- verified

People

(Reporter: njn, Assigned: emilio)

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is
report bp-0d4a3038-322b-43d7-8f46-e9de20171203.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll mozilla::ServoStyleSet::SetStylistStyleSheetsDirty layout/style/ServoStyleSet.cpp:1050
1 xul.dll mozilla::StyleSheet::RuleRemoved layout/style/StyleSheet.cpp:641
2 xul.dll mozilla::ServoStyleSheet::DeleteRuleInternal layout/style/ServoStyleSheet.cpp:440
3 xul.dll mozilla::StyleSheet::DeleteRule layout/style/StyleSheet.cpp:588
4 xul.dll mozilla::dom::CSSStyleSheetBinding::deleteRule dom/bindings/CSSStyleSheetBinding.cpp:223
5 xul.dll mozilla::dom::GenericBindingMethod dom/bindings/BindingUtils.cpp:3042
6 xul.dll js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:473
7 xul.dll Interpret js/src/vm/Interpreter.cpp:3096
8 xul.dll js::RunScript js/src/vm/Interpreter.cpp:423
9 xul.dll js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:495

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

The crash address is always 0x20, and it occurs on this line:

mPresContext->RestyleManager()->AsServo()->IncrementUndisplayedRestyleGeneration();

So it's probably a null pointer somewhere in that dereference chain.

heycam, any ideas?
Flags: needinfo?(cam)
Assuming no funny business with deleted objects, we must be in a situation where:

* the StyleSheet still has this ServoStyleSet in its mStyleSets array
* the nsPresContext has had its mRestyleManager cleared, which means we've called nsPresContext::DetachShell
* we must have called PresShell::Destroy, which is what calls nsPresContext::DetachShell
* but if the ServoStyleSet still exists, we can't have destroyed the PresShell object yet (since it is deleted in the PresShell destructor)

It's conceivable the PresShell is lasting a bit longer than we expect, since it's a COM object and other things might have a strong reference to it.  (Though we do try to break some of those references, apparently, in nsDocumentViewer::DestroyPresShell.)

If this is all correct (and reasonable), then we could start recording mInShutdown in ServoStyleSet, and just return early from these style sheet dirty methods when it's true.

Does that sound reasonable Emilio?
Flags: needinfo?(cam) → needinfo?(emilio)
Yup. I have another idea, given we need to support null pres contexts anyway for XBL / Shadow DOM... But we should really fix bug 154199...
Assignee: nobody → emilio
Flags: needinfo?(emilio)
Comment on attachment 8934118 [details]
Bug 1422634: Avoid sheet notifications to dereference a null restyle manager.

https://reviewboard.mozilla.org/r/205058/#review210526

This works too. :-)  Please mention when mPresContext becomes null in the comment on its declaration.
Attachment #8934118 - Flags: review?(cam) → review+
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/5091ed61e147
Avoid sheet notifications to dereference a null restyle manager. r=heycam
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e1b04d5511d5
followup: fix typo in the comment. r=comment-only
https://hg.mozilla.org/mozilla-central/rev/5091ed61e147
https://hg.mozilla.org/mozilla-central/rev/e1b04d5511d5
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
No more crashes in nightly 59 since the patch landed.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.