1615832 - Crash in [@ mozilla::css::StreamLoader::~StreamLoader]

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[[:philipp]]

This bug is for crash report bp-88d4a8a3-fd6e-4e73-a8d4-4b8ec0200215.

Top 10 frames of crashing thread:

0 xul.dll mozilla::css::StreamLoader::~StreamLoader layout/style/StreamLoader.cpp:24
1 xul.dll mozilla::css::StreamLoader::~StreamLoader layout/style/StreamLoader.cpp:23
2 xul.dll mozilla::EditorEventListener::Release editor/libeditor/EditorEventListener.cpp:293
3 xul.dll mozilla::css::Loader::LoadSheet layout/style/Loader.cpp
4 xul.dll mozilla::css::Loader::LoadChildSheet layout/style/Loader.cpp:2236
5 xul.dll LoadImportSheet layout/style/GeckoBindings.cpp:1537
6 xul.dll Gecko_LoadStyleSheet layout/style/GeckoBindings.cpp:1577
7 xul.dll geckoservo::stylesheet_loader::{{impl}}::request_stylesheet servo/ports/geckolib/stylesheet_loader.rs:61
8 xul.dll cssparser::rules_and_declarations::parse_at_rule<style::stylesheets::rule_parser::TopLevelRuleParser, style_traits::StyleParseErrorKind> third_party/rust/cssparser/src/rules_and_declarations.rs:475
9 xul.dll static style::stylesheets::stylesheet::Stylesheet::parse_rules servo/components/style/stylesheets/stylesheet.rs:491

this crash is appearing in nightly build 20200215095617 with MOZ_DIAGNOSTIC_ASSERT(mOnStopRequestCalled || mAsyncOpenFailed).

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Thanks for filing! I'll follow-up in the bug that added the assertion.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

This is a bug in the diagnostic patch in bug 1605895, so I'll address here instead.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1615832 - Fix diagnostics for error paths in sync loads.

This can happen with invalid @import rules in userContent / userChrome.css

Comment 4

[Emilio Cobos Álvarez (:emilio)]

I'm going to land this because:

• It's a pretty annoying / hard to diagnose startup crash.
• There's no changes for release builds, only for nightly.

Cam, could you sanity-check it? We don't really care about sync loads for the purposes of diagnosing bug 1605895.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/361b2704db9f
Fix diagnostics for error paths in sync loads.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1615832 - Fix a test after the previous patch.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a41e6a90419d
Fix a test after the previous patch.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1615832 - Add a missing file for the test for this bug... d'oh.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ba1a49bfe104
Add a missing file for the test for this bug... d'oh.

Comment 10

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/361b2704db9f
https://hg.mozilla.org/mozilla-central/rev/a41e6a90419d
https://hg.mozilla.org/mozilla-central/rev/ba1a49bfe104

Comment 11

[Calixte Denizet (:calixte)]

There still are some crashes after the patch landed: https://mzl.la/2STz1rd
The moz_crash_reason is always: MOZ_DIAGNOSTIC_ASSERT(mOnStopRequestCalled || mChannelOpenFailed)
:emilio, could you have a look ?

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Yeah, that's actually the bug we were looking for. Will nag the necko people in the other bug.

Comment 13

[Emilio Cobos Álvarez (:emilio)]

Pascal, is this a topcrash with the fix in this bug? it seems from the link in comment 11 there's only been 12 crashes since it landed.

Comment 14

[User Dderss]

Attachment: Some pinned tabs get absolutely erased forever without a trace.png

This bug is said to be fixed for FF 75, but I am still getting it regularly: https://crash-stats.mozilla.org/report/index/4d55b6d5-8cb4-4227-aa81-ca3530200314

What is even worse, it not only generates a complete crash-hang of FF but it also consistently corrupts session, killing some pinned tabs -- see a few of placeholder globe favicons after the restart. This is quite annoying since I can not always remember what was there.

Can somebody please look at my crash report to see if it is really the same bug or a new one?
Thanks in advance.

Comment 15

[Julien Cristau [:jcristau]]

There's many crashes now in 75beta.

Comment 16

[Emilio Cobos Álvarez (:emilio)]

That crash is different from the one fixed here, and is tracked in bug 1605895.

I'll nag the necko people there... But is there any chance you could attach your about:support? May you have extensions that may modify stylesheets or something that could help reproduce this?

Comment 17

[User Dderss]

Attachment: AboutSupport

Comment 18

[User Dderss]

Unfortunately, there is no clear pattern of action to reproduce this but it happens mainly when I click on a video link in a YT notification mail in Thunderbird. The tab opens but it immediately crashes. But it does not happen every time.

Comment 19

[Firefox Bug Husbandry Bot]

Please specify a root cause for this bug. See :tmaity for more information.

Comment 20

[User Dderss]

Emilio, can you please check progress in #bug 1605895?

This bug is security-sensitivity so I can not see it but I wonder why it is not fixed yet for so long, especially since it is quite a nasty bug (that also yanks some of my tabs every time, which did not happen in FF for years already) with well over thousand of crashes in recent weeks, a serious regression over FF 74 which did not have such issues at all.

Thanks in advance.

Comment 21

[Emilio Cobos Álvarez (:emilio)]

So I thought that MOZ_DIAGNOSTIC_ASSERT was disabled on dev-edition / etc. This assertion should be disabled on the release channel so it shouldn't cause crashes there... but apparently is unconditionally enabled in dev-edition which seems unfortunate.

There is progress from the networking folks in that bug as far as I can tell. But I'll remove this assertion for now as they served their purpose already.

Comment 22

[Emilio Cobos Álvarez (:emilio)]

btw, if you can reproduce this somewhat frequently by all means tell me how :)

Comment 23

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1615832 - Make an assertion nightly-only.

We confirmed that this is a networking issue, no point in making beta /
devedition crash due to this. Still worth keeping it on nightly to monitor the
crash volume once the networking fix arrives.

Comment 24

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9138110 [details]
Bug 1615832 - Make an assertion nightly-only.

Beta/Release Uplift Approval Request

• User impact if declined: unnecessary crashes for beta / devedition users.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Just downgrading a diagnostic assertion so that we don't crash. The lack of the assertion itself doesn't cause any security issues.
• String changes made/needed: none

Comment 25

[User Dderss]

Alas, I can not reproduce it via a step-by-step method but the crashes happen only when I open links in new tabs, never during any other action. Crashes kill one of the multiple firefox.exe processes along with a few tabs (pinned, at least in my case).

But the issue is it does not happen on every action of opening a link in a new tab, it only happens once in a few dozens of such actions resulting in crashes happening only once or twice a day despite very active use of the browser. This forbids me from recording the bug (capturing minidump) at least in a regular WinDbg mode which generates a giant file very quickly (maybe I could do it if I had Mac Pro with 1.5 TB of RAM though).

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7c9ac7d5bef4
Make an assertion nightly-only.

Comment 27

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/7c9ac7d5bef4

Comment 28

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9138110 [details]
Bug 1615832 - Make an assertion nightly-only.

76 is on Beta now.

Comment 29

[Julien Cristau [:jcristau]]

Comment on attachment 9138110 [details]
Bug 1615832 - Make an assertion nightly-only.

diagnostic asserts are off on release, I think we're fine here.

Comment 30

[Jean-Yves Avenard [:jya]]

I can reproduce the crash with 100% certainty as follow.

On Android, in any application using an embedded browser such as slack or nine mail client.

Select "open in Firefox Nightly"

It will crash, guaranteed

Latest crash report: bp-429e9dd8-04d6-4ab2-b4f3-e8cf20200903

Comment 31

[Jean-Yves Avenard [:jya]]

Attachment: Screenshot_20200903-154924.png

Comment 32

[Jean-Yves Avenard [:jya]]

Probably something else.