598851 - HUDService's ResponseListener eats all my memory

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Matthew Gregan [:kinetik]]

Activating Web Console on a page that starts fetching a large resource over HTTP (e.g. a 1GB video) results in significant memory consumption due to HUDService's ResponseListener storing a copy of the incoming data in a string.  If I right-click on the network log message in the console, "Log Request and Response Bodies" is not checked.

Comment 1

[David Dahl :ddahl]

(In reply to comment #0)
> Activating Web Console on a page that starts fetching a large resource over
> HTTP (e.g. a 1GB video) results in significant memory consumption due to
> HUDService's ResponseListener storing a copy of the incoming data in a string. 
> If I right-click on the network log message in the console, "Log Request and
> Response Bodies" is not checked.

What, No patch?

Comment 2

[David Dahl :ddahl]

(In reply to comment #0)
> Activating Web Console on a page that starts fetching a large resource over
> HTTP (e.g. a 1GB video) 

We will need to set a max size when checking requests where we discard the content. Or perhaps by content-type? I imagine some JPEGs will also eat memory pretty badly.

Comment 3

[Rob Campbell [:rc] (:robcee)]

Does the fix in bug 597502 help at all? E.g., could you retest with the latest minefield nightly for your system?

(e.g., http://nightly.mozilla.org/)

Also, some additional test case information (like an example URL) would be helpful.

thanks for the report!

Comment 4

[Matthew Gregan [:kinetik]]

That bug sounds like the same problem, but it's not.  In this case, the problem is simply the amount of data accumulated in the receiveData string on the ResponseListener as it arrives.  Discarding it after the request has finished (as bug 597502 does) doesn't help because, with a large enough request, it's possible to run out of memory before the request completes.

Here's an easy way to reproduce:

1. Serve a large file and a page containing a download link with a local HTTP server.  I created a large fake zip file using: dd if=/dev/zero of=huge.zip bs=102400 count=10240

2. Visit download page.  I used:
<a href="huge.zip">download me</a>

3. Open Web Console.

4. Click download link.

5. Watch Firefox's RSS (private memory) grow to ~2.3GB.

Comment 5

[Rob Campbell [:rc] (:robcee)]

there may be something to this. I just tried duplicating your test and watched my memory balloon up to about 2GB and then firefox crashed.

crash report here:

http://crash-stats.mozilla.com/report/index/bp-29f38ad4-fbfe-4cfc-86d6-1add42100924

4.0b6, MacOS X 10.6

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b6) Gecko/20100101 Firefox/4.0b6

Comment 6

[Rob Campbell [:rc] (:robcee)]

looks like that crash may have been fixed on nightlies. Need more testing there. Regardless, this is going to be a problem. It did seem to clean up after itself during a previous run when I canceled the download and closed the tab. Memory dropped back to what it was before the test.

Comment 7

[Rob Campbell [:rc] (:robcee)]

requesting blocking final.

Comment 8

[Mihai Sucan [:msucan]]

Attachment: proposed patch

Proposed patch.

Once I apply this, I can't see any more problems with memory usage. I can download, say, an Ubuntu CD ISO without memory usage spikes.

The test code change I did is ... minimal, because the httpActivity.response.listener object is now emptied once a request stops. So, I can't check if receivedData is emptied or not. I think just checking if response.listener is cleared is enough.

Suggestions for improvements are welcome!

Comment 9

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 481546 [details] [diff] [review]
proposed patch

I don't see much to improve there. That's pretty much how I was going to do it.

Comment 10

[Mihai Sucan [:msucan]]

Comment on attachment 481546 [details] [diff] [review]
proposed patch

Thanks Robert for your feedback+!

Asking for review from Dietrich.

Comment 11

[Mihai Sucan [:msucan]]

Thanks Dietrich for the review+!

Comment 12

[Mihai Sucan [:msucan]]

Attachment: rebased patch

Rebased patch. One liner change, hehe.

Comment 13

[David Dahl :ddahl]

http://hg.mozilla.org/mozilla-central/rev/4ed717efe8f4