Closed Bug 1422634 Opened 3 years ago Closed 3 years ago

Crash in mozilla::ServoStyleSet::SetStylistStyleSheetsDirty


(Core :: CSS Parsing and Computation, defect)

Windows 10
Not set



Tracking Status
firefox-esr52 --- unaffected
firefox57 --- unaffected
firefox58 --- unaffected
firefox59 --- verified


(Reporter: njn, Assigned: emilio)


(Keywords: crash)

Crash Data


(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:


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.

This works too. :-)  Please mention when mPresContext becomes null in the comment on its declaration.
Attachment #8934118 - Flags: review?(cam) → review+
Pushed by
Avoid sheet notifications to dereference a null restyle manager. r=heycam
Pushed by
followup: fix typo in the comment. r=comment-only
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
No more crashes in nightly 59 since the patch landed.
You need to log in before you can comment on or make changes to this bug.