Closed Bug 994740 Opened 10 years ago Closed 10 years ago

XML parsing error in innerHTML setter causes a large leak

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Platform:
x86_64
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla31
Tracking Flags:
Tracking Status
firefox29 + wontfix
firefox30 + fixed
firefox31 + fixed
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: jruderman, Assigned: smaug)

References

Details

(Keywords: memory-leak, regression, testcase, Whiteboard: [MemShrink])

Attachments

(2 files)

testcase
220 bytes, application/xhtml+xml
Details
fix + test
1.66 KB, patch
hsivonen
: review+
lsblakk
: approval-mozilla-aurora+
Details | Diff | Splinter Review
Attached 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.
Attached patch fix + test — — Splinter 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?

          status-firefox29:
          --- → affected

          status-firefox30:
          --- → affected

          tracking-firefox29:
          --- → ?

          tracking-firefox30:
          --- → ?
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+

    
    

    
  
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: 10 years ago

          status-firefox31:
          affected → fixed
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.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: