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

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Bob Clary [:bc] (inactive)]

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

Comment 1

[Boris Zbarsky [:bzbarsky]]

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...

Comment 2

[Boris Zbarsky [:bzbarsky]]

Oh, and Chrome's content process goes out to lunch for a while with 100% CPU on this page.  And then crashes.

Comment 3

[Mats Palmgren (inactive)]

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.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Go for it!

Comment 5

[Mats Palmgren (inactive)]

Attachment: 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.

Comment 6

[Mats Palmgren (inactive)]

Attachment: 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 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8683762 [details] [diff] [review]
fix

r=me.  Want to add a crashtest too?

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8683764 [details] [diff] [review]
crashtest

Ah, here's the crashtest.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3c1cb9e4546c
https://hg.mozilla.org/integration/mozilla-inbound/rev/534bae0f55bc

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3c1cb9e4546c
https://hg.mozilla.org/mozilla-central/rev/534bae0f55bc

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I backed out the crashtest from 534bae0f55bc at the request of mats.
https://hg.mozilla.org/integration/mozilla-inbound/rev/5ac00a7965de

Comment 12

[Arthur K. (he/him)]

This signature seems to be back. Just had it happen on 101.0b2: https://crash-stats.mozilla.org/report/index/78f70879-b072-460d-a230-c11d90220516

Comment 13

[Arthur K. (he/him)]

Created bug 1769635 for the newer crash signature (mozilla::css::SheetLoadData::~SheetLoadData)