crash in mozilla::css::SheetLoadData::`scalar deleting destructor''

RESOLVED FIXED in Firefox 45

Status

()

--
critical
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: bc, Assigned: mats)

Tracking

(Blocks: 1 bug, {crash})

43 Branch
mozilla45
x86
Windows NT
crash
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox45 fixed)

Details

(crash signature)

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
This bug was filed from the Socorro interface and is 
report bp-ee91226b-4a87-4b30-a8c6-3c3762151105.
=============================================================

1. https://wpmention.com/wp-theme/reales-wp-real-estate-wordpress-theme/

2. bp-ee91226b-4a87-4b30-a8c6-3c3762151105

EXCEPTION_STACK_OVERFLOW Crash [@ mozilla::css::SheetLoadData::`scalar deleting destructor'' ]

Reproduced in Bughunter on Beta/43, Aurora/44, Nightly/45 with variations of the stack
This is exciting.

The page loads https://wpmention.com/wp-content/cache/wpfc-minified/8d59bbe394e079e027b24b31e5092e9b/1437615250index.css which starts with 32768 (a highly suggestive number!) lines, each of which is:

@import url("https://fonts.googleapis.com/css?family=Ubuntu:300,400,500,700|Roboto:400,300,500,700");

That means that we get a chain of 32768 SheetLoadDatas all for the same load; it's a linked list.  Then we go to delete it, do it recursively, and end up with a stack overflow.

I guess we should try deleting it iteratively instead...
Component: Layout → CSS Parsing and Computation
Oh, and Chrome's content process goes out to lunch for a while with 100% CPU on this page.  And then crashes.
(Assignee)

Comment 3

3 years ago
I can take a look if you're not already on it Boris.  I seem to recall writing macros
for this sort of thing a few years back.
Assignee: nobody → mats
(Assignee)

Comment 5

3 years ago
Created attachment 8683762 [details] [diff] [review]
fix

The existing macro uses 'delete' though and in this case we want NS_RELEASE:
http://mxr.mozilla.org/mozilla-central/source/layout/style/nsCSSValue.h#41
so I just copy-pasted it and changed that line.

I guess we could add a param to the existing macro take and pass in
delete/NS_RELEASE at each call site instead, if you prefer.
Attachment #8683762 - Flags: review?(bzbarsky)
(Assignee)

Comment 6

3 years ago
Created attachment 8683764 [details] [diff] [review]
crashtest

This test is a tad slow: 15 sec in my debug Linux build, but I guess it's OK.
(we already have a crashtest in this directory that takes 25 sec)

https://treeherder.mozilla.org/#/jobs?repo=try&revision=42f80f72dbab
Comment on attachment 8683762 [details] [diff] [review]
fix

r=me.  Want to add a crashtest too?
Attachment #8683762 - Flags: review?(bzbarsky) → review+
Comment on attachment 8683764 [details] [diff] [review]
crashtest

Ah, here's the crashtest.
(Assignee)

Updated

3 years ago
Flags: in-testsuite+
(Assignee)

Updated

3 years ago
Depends on: 1222287

Comment 10

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3c1cb9e4546c
https://hg.mozilla.org/mozilla-central/rev/534bae0f55bc
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
I backed out the crashtest from 534bae0f55bc at the request of mats.
https://hg.mozilla.org/integration/mozilla-inbound/rev/5ac00a7965de
You need to log in before you can comment on or make changes to this bug.