Closed Bug 1422634 Opened 2 years ago Closed 2 years ago
Crash in mozilla::Servo
Style Set::Set Stylist Style Sheets Dirty
59 bytes, text/x-review-board-request
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?
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
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 firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/5091ed61e147 Avoid sheet notifications to dereference a null restyle manager. r=heycam
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/e1b04d5511d5 followup: fix typo in the comment. r=comment-only
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.