473911 - Crash due to too much recursion in nsCSSDocumentRule::URL::~URL

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Jesse Ruderman]

Attachment: testcase (a little slow; crashes when closed)

http://mxr.mozilla.org/mozilla-central/source/layout/style/nsCSSRules.h#209

~URL() { delete next; }

Should probably be converted into a loop, either in the destructor or in whatever causes the destructor to run.

Comment 1

[Jeff Walden [:Waldo]]

Attachment: Loop inside ~URL

Replacing all delete calls with ->Destroy() calls would perhaps be aesthetically better (and would avoid a next-null check if the compiler's not super-clever), but then you can't have automatic destruction of nsCSSDocumentRule::mURLs, and the patch would touch more lines.  I prefer this given the more-lines concern, but your call which is better.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Rather than adding a Destroy function, could you just use the NS_CSS_DELETE_LIST_MEMBER macro?  This also has the advantage of making it easier to find all the places where we do this.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Also, it seems like the test should instead go in layout/style/crashtests/

Comment 4

[Jeff Walden [:Waldo]]

Attachment: Done and done

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 357437 [details] [diff] [review]
Done and done

r+sr=dbaron

Comment 6

[Jeff Walden [:Waldo]]

http://hg.mozilla.org/mozilla-central/rev/0cb4f7b53ca0