Closed Bug 1221902 Opened 4 years ago Closed 4 years ago

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

Categories

(Core :: CSS Parsing and Computation, defect, critical)

43 Branch
x86
Windows NT
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: bc, Assigned: mats)

References

(Blocks 1 open bug)

Details

(Keywords: crash)

Crash Data

Attachments

(2 files)

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.
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
Attached patch fixSplinter Review
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)
Attached patch crashtestSplinter Review
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.
Flags: in-testsuite+
Depends on: 1222287
https://hg.mozilla.org/mozilla-central/rev/3c1cb9e4546c
https://hg.mozilla.org/mozilla-central/rev/534bae0f55bc
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.