Closed Bug 473911 Opened 11 years ago Closed 11 years ago

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

Categories

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

defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla1.9.2a1

People

(Reporter: jruderman, Assigned: Waldo)

References

(Blocks 1 open bug)

Details

(Keywords: crash, testcase)

Attachments

(2 files, 1 obsolete file)

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.
Attached patch Loop inside ~URL (obsolete) — Splinter Review
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.
Assignee: nobody → jwalden+bmo
Status: NEW → ASSIGNED
Attachment #357333 - Flags: review?(dbaron)
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.
Attachment #357333 - Flags: review?(dbaron) → review-
Also, it seems like the test should instead go in layout/style/crashtests/
Attached patch Done and doneSplinter Review
Attachment #357333 - Attachment is obsolete: true
Attachment #357437 - Flags: review?(dbaron)
Comment on attachment 357437 [details] [diff] [review]
Done and done

r+sr=dbaron
Attachment #357437 - Flags: superreview+
Attachment #357437 - Flags: review?(dbaron)
Attachment #357437 - Flags: review+
http://hg.mozilla.org/mozilla-central/rev/0cb4f7b53ca0
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
OS: Mac OS X → All
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
Blocks: 476744
Depends on: 477490
You need to log in before you can comment on or make changes to this bug.