1719963 - Crash in [@ style::stylesheets::import_rule::ImportSheet::rules]

Status: RESOLVED FIXED

(Core :: DOM: CSS Object Model, defect)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

5 crashes from 5 installations (both Windows and macOS), all with the latest Nightly version (91.0a1 20210709214006).

This should be from bug 1711437.

Crash report: https://crash-stats.mozilla.org/report/index/e585cc03-602b-4d44-a8f5-e92b90210710

Reason: EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames of crashing thread:

0 xul.dll style::stylesheets::import_rule::ImportSheet::rules servo/components/style/stylesheets/import_rule.rs:73
1 xul.dll style::stylesheets::rules_iterator::{{impl}}::next<style::stylesheets::rules_iterator::EffectiveRules> servo/components/style/stylesheets/rules_iterator.rs:132
2 xul.dll style::stylist::CascadeData::add_stylesheet<style::gecko::data::GeckoStyleSheet> servo/components/style/stylist.rs:2169
3 xul.dll style::stylist::CascadeData::rebuild<style::gecko::data::GeckoStyleSheet> servo/components/style/stylist.rs:2025
4 xul.dll geckoservo::glue::Servo_StyleSet_FlushStyleSheets servo/ports/geckolib/glue.rs:1938
5 xul.dll mozilla::ServoStyleSet::UpdateStylist layout/style/ServoStyleSet.cpp:1193
6 xul.dll mozilla::PresShell::DoFlushPendingNotifications layout/base/PresShell.cpp:4159
7 xul.dll nsRefreshDriver::Tick layout/base/nsRefreshDriver.cpp:2296
8 xul.dll mozilla::RefreshDriverTimer::TickRefreshDrivers layout/base/nsRefreshDriver.cpp:326
9 xul.dll mozilla::RefreshDriverTimer::Tick layout/base/nsRefreshDriver.cpp:342

Comment 1

[Alexandru Michis [:malexandru]]

Should be fixed by the backout of Bug 1711437:
https://hg.mozilla.org/mozilla-central/rev/7b8721ff033aa37d5036caa4d2bfa5127356dd62

Comment 2

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Here we go again.

Comment 3

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Fixed by backout of bug 1711437.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Do you know if we had any ASAN reports that could look like this, or if the fuzzers hit this these last couple days?

Comment 5

[Christian Holler (:decoder)]

Nothing I could see in ASan Nightly.

Comment 6

[Jason Kratzer [:jkratzer]]

Nothing reported by the fuzzers either.

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

20-50 crash reports per Nightly.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Hmm, so the null pointer is not the one I thought. Will fix.

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Report of a newer build for reference: https://crash-stats.mozilla.org/report/index/f3359a55-6681-4b39-a299-b5b500210722

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1719963 - Better wallpaper for now.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4b53d6459297
Better wallpaper for now.

Comment 12

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/4b53d6459297

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Backed out for causing topcrash bug 1722150.
https://hg.mozilla.org/mozilla-central/rev/03637911a02a3d366a3f66869bbb76c7626a55ee

Comment 14

[Ryan VanderMeulen [:RyanVM]]

We'll need to keep an eye on the crash rate for Beta92 and may need to backout bug 1711437 from there as well depending on the frequency.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

I went ahead and did the backout for 92.0b2. Nightly still has bug 1711437 on it, however.
https://hg.mozilla.org/releases/mozilla-beta/rev/0b48d14c2361

Comment 16

[Julien Cristau [:jcristau]]

I think we should go ahead and back out 1711437 from nightly 93 at this point.

Comment 17

[Emilio Cobos Álvarez (:emilio)]

I think I just found a repro thanks to Bomsy (https://phabricator.services.mozilla.com/D122327 causes this bug to show up on automation), can we wait one more day please?

Comment 18

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1719963 - Fix CSSImportRule CC setup. r=jfkthame

Right now, CSSImportRule was reporting two references to its child
sheet, one from mChildSheet, one from mRawRule. mRawRule however
is kept alive by the StyleSheetContents, so it's more correct to
report it from TraverseInner instead, and ensure that
DropSheetReference takes care of also dropping
CSSImportRule::mRawRule.

Comment 19

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1719963 - Restore EnsureUniqueInner() call for now. r=jfkthame

This effectively reverts the behavior to the one before bug 1711437
(making the CC setup sound again), but without a big backout.

Fixing the CC setup of @import rules properly is a bit more involved
than what I anticipated and I don't want to have DevTools folks blocked
for too long, nor have this crash in-tree for too long either.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0dffe73ea4df
Restore EnsureUniqueInner() call for now. r=jfkthame

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2fc1d893c851
Followup so that getMatchedCSSRules keeps working.

Comment 22

[Cristina Cozmuta (:CrissCozmuta)]

https://hg.mozilla.org/mozilla-central/rev/0dffe73ea4df
https://hg.mozilla.org/mozilla-central/rev/2fc1d893c851