XML parsing error in innerHTML setter causes a large leak

RESOLVED FIXED in Firefox 30

Status

()

defect
RESOLVED FIXED
5 years ago
5 months ago

People

(Reporter: jruderman, Assigned: smaug)

Tracking

(Blocks 1 bug, {memory-leak, regression, testcase})

Trunk
mozilla31
x86_64
macOS
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox29+ wontfix, firefox30+ fixed, firefox31+ fixed, b2g-v1.4 fixed, b2g-v2.0 fixed)

Details

(Whiteboard: [MemShrink])

Attachments

(2 attachments)

Posted file testcase
No description provided.
What do we leak?
Flags: needinfo?(jruderman)
A document and stuff around it.
Assignee: nobody → bugs
This is a regression? Do you know when this regressed ?
I'd say bug 380028, but that is old.
Posted patch fix + testSplinter Review
Attachment #8404877 - Flags: review?(hsivonen)
Whiteboard: [MemShrink]
I guess this is a regression from Bug 596182.
The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/c6ccf0698fa1
user:        Bobby Holley
date:        Tue Apr 08 15:51:33 2014 -0700
summary:     Bug 913138 - Release nsLayoutStatics when the layout module is unloaded. r=bsmedberg
Blocks: 913138
Flags: needinfo?(jruderman)
Is this a shutdown-only leak? Bug 913138 landed into aurora/beta... do we need to track this or consider backing that out?
Flags: needinfo?(bugs)
I think Bug 913138 just made this visible. As far as I see, the leak has been there, just
explicitly cleared before shutdown.
Flags: needinfo?(bugs)
Attachment #8404877 - Flags: review?(hsivonen) → review+
Keywords: checkin-needed
Olli, can we get an uplift request? I would like to have it for beta 8. thanks
Flags: needinfo?(bugs)
Hmm, was the test somehow wrong.
No, the test was fine, but started to fail after some other change.
Flags: needinfo?(bugs)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/e799049584b6
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Olli - now that this has landed properly, can you nominate for uplift with risk assessment?  It looks like  a pretty minor fix so maybe we'll be able to consider it for the final FF29 builds and not ship this perf regression at all.
Flags: needinfo?(bugs)
Correction, the regression might be older than that - if this is not a trivial patch to uplift, we'll just get it to Aurora and ride the trains from there.
As far as I see, this is quite old stuff, so I don't quite see the reason for beta.
Flags: needinfo?(bugs)
Comment on attachment 8404877 [details] [diff] [review]
fix + test

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 380028, but  Bug 913138 made this visible
User impact if declined: Leaks, higher cycle collection pauses 
Testing completed (on m-c, etc.): landed to m-C
Risk to taking this patch (and alternatives if risky): Shouldn't be too risky 
String or IDL/UUID changes made by this patch: NA
Attachment #8404877 - Flags: approval-mozilla-aurora?
Attachment #8404877 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.