233579 - XSLT memory leak

Status: RESOLVED FIXED

(Core :: XSLT, defect, P3)

Description

[Andras Ketskes]

User-Agent:       
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7a) Gecko/20040209

While developing a web based application doing XSLT on the client side I came
across this bug:
There seems to be a memory leak when processing XML docs with XSLT. It's quite
hard to notice if file sizes are small or pages don't get reloaded too often.
The test case has a tiny XML file and a relatively large XSL. All it does is
generate a simple HTML page and when it's loaded, start over again using
window.location.reload(). You will see more and more memory consumed by Mozilla
while the stuff's running. The process can be accelerated by downloading the
files and running them localy. Once it consumes all the memory it carries on
running, but the browser becomes slow and irresponsive (Windows tolerates this
even whorse).
The original app. btw submitted forms with the same result.

Reproducible: Always
Steps to Reproduce:
1. open testcase
2. lean back and watch your memory go away... :)


Actual Results:  
All the memory was allocated by Mozilla, but it didn't crash.

Expected Results:  
It should free the memory allocated for parsing after it's finished, so that the
memory consumption of reloading the same page could be constant.

Comment 1

[Axel Hecht]

It may be that this is calling onload without having teared down XSLT. Ending
up in a loop, if cache kicks in and we process synchronously. But that's just
a guess.
If that'd be the case, this might get fixed by bug 205778. Though, thinking
about it, I'd be puzzled somehow, as this doesn't seem to conflict with HTTP
cache, which should happen in synch mode. Or not? Bah, events.

Comment 2

[Andras Ketskes]

(In reply to comment #1)
It can't be the event (that's only for introduction purposes). Like I said, the
original app. used forms. It sent the XML, Mozilla parsed it to HTML, showed the
HTML form, users filled it in and pressed submit. No js events at all.
I only used the onload->reload to speed up page reloading so that the memory
consumption becomes easier to notice. You can achieve the same results by
manually reloading the page with the reload button. Or you may use a form with a
send button to reload and reparse.
I guess it's (some part of) the XSL stylesheet that doesn't get freed or maybe
something is left behind from the parsing. (But that's just a guess.)

Comment 3

[Andras Ketskes]

Attachment: Test case to reproduce the bug

Comment 4

[Andras Ketskes]

Attachment: Leak trace

I have used the leaktracer tool to find the leaks with this results. What I did
was: start mozilla, open the directory (from the local disk) where a testcase
is, open the xml file, so that the browser parses it. This file didn't have the
onload->reload() stuff in the html body, so it only got parsed once.
I don't know if this would help (or if the results are even correct), but I
guess it's worth a look.

Comment 5

[Andras Ketskes]

Ooops, I forgot to say, that this is for Mozilla 1.4.1 as that was the one i
could get to compile. I guest that this will be no problem as the bug is present
in the latest versions, too. (I've tested that with the binaries from mozilla.org.)

Comment 6

[Peter Van der Beken [:peterv]]

Attachment: v1

This makes us release the parser (held in mListener) from the txStylesheetSink
once we don't need it anymore. Andras: could you verify if this helps?

Comment 7

[Axel Hecht]

Comment on attachment 141148 [details] [diff] [review]
v1

peter tested this pretty good *and* it makes sence. r=me

Comment 8

[Andras Ketskes]

This seems to do the job! I'm testing it furter, but it seems to have solved the
problem. More on this soon...

Comment 9

[Andras Ketskes]

Attachment: Trace of patched build

Well done! This issue is fixed. As you can see in the trace, there's only a
tiny leak at the beginnig, but it's fine after that. This leak only happens
once, no matter how many times you parse the page. Maybe this should be fixed
to, but it doesn't seem to be too serious.
Thanks for the quick response!

Comment 10

[Darin Fisher]

Comment on attachment 141148 [details] [diff] [review]
v1

sr=darin

Comment 11

[Peter Van der Beken [:peterv]]

Comment on attachment 141148 [details] [diff] [review]
v1

Simple leak fix by breaking mutual strong ownership when it's not needed
anymore.

Comment 12

[chris hofmann]

Comment on attachment 141148 [details] [diff] [review]
v1

a=chofmann