Closed Bug 290969 Opened 19 years ago Closed 19 years ago

[FIX]XmlHttpRequest leaks memory (all) and eventually crashes (OS X and Linux)

Categories

(Core :: Networking, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla1.8beta2

People

(Reporter: bill, Assigned: bzbarsky)

Details

(Keywords: memory-leak, testcase)

Attachments

(2 files)

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en) AppleWebKit/405 (KHTML, like Gecko) Safari/405
Build Identifier: FireFox 1.0.0, 1.0.1, 1.0.2, nightly

When doing AJAX style pages with high density XmlHttpRequests (one per second), FireFox leaks memory 
and will eventually (hour or two) crash on Linux and OS X. On Windows, it just slows down. Setting the 
cache size to 0 'seems' to work around the problem, at least on OS X. I have reduced the test case to the 
attached HTML page.

Reproducible: Always

Steps to Reproduce:
1. Place test page on a local Web server under http://.../loadtest.html
2. Hit the page with a Mozilla based browser
3. What memory usage

Actual Results:  
Leaks about 3kb per request (OS X) unless cache is set to 0. If cache size is not set to zero, the browser 
will eventually crash on OS X and Linux.
Attached file HTML Test Case
This looks like bug 280808. Reporter, are you sure that the bug also exists in
the trunk-version ?
Keywords: testcase
Might be a dup of 280808, but I still see it on...

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050417 Firefox/1.0+

As an additional note, I wasn't quite correct about setting the cahce size to 0 as a work around. A had 
clared the cache as well. Even if the cache size is set to 0, clearing seems to be releasing something. 
Eventually the leak catches up to the currently allocated memory size and starts increasing again.

I'd be happy to provide any other info that might be helpful.
Keywords: mlk
So the issue here is that the requests are made sync and off a 100ms interval
timer.  Since they're sync, we push a new event queue while the request is in
progress.  But the timer fires before the request completes, and uses the
current event queue, not the original one (I thought we had a bug on this, but I
can't find it....).

As a result, I get things like this in a debug build:

WARNING: event queue chain length is 43. this is almost certainly a leak., file
../../../mozilla/xpcom/threads/nsEventQueue.cpp, line 553

Interestingly, opening a new window reduces the event queue chain length back to
something near zero, and then it starts increasing again.  Not sure why that
happens, exactly.
Status: UNCONFIRMED → NEW
Ever confirmed: true
So it looks like I was partly wrong in blaming this on the interval timers... at
least for the case I was testing (which has a pretty immediate 404 from the
server, since I was just running it against bugzilla).

The reason opening a new window helps is because it GCs.  This kills the old
XMLHttpRequest object, which makes it drop its reference to the channel, which
makes it drop its reference to the event queue.  That allows the event queue to
go away.

I'm guessing we'd GC eventually anyway, once we had about 4000 event queues on
the list.  But since it's a linked list, that could make things pretty slow first.

I'll attach a patch to make the HTTP channel drop the event queue when it's
done; that fixes the chain-length-growing for me.
Attached patch PatchSplinter Review
I'll work on setting up a server that I can test this on with slow POST
responses... see how that goes.
Attachment #183800 - Flags: superreview?(darin)
Attachment #183800 - Flags: review?(darin)
William: The workaround for this bug is to use the "async" mode of loading pages
using XMLHttpRequest.  This is especially important if you are building webapps
for users over slow connections.
Attachment #183800 - Flags: superreview?(darin)
Attachment #183800 - Flags: superreview+
Attachment #183800 - Flags: review?(darin)
Attachment #183800 - Flags: review+
Comment on attachment 183800 [details] [diff] [review]
Patch

Requesting 1.8b2 approval.  Simple fix to not hold on to event queues after we
don't need them anymore, since that keeps them from going away.
Attachment #183800 - Flags: approval1.8b2?
I put a testcase that actually takes a long time to respond at
http://landfill.mozilla.org/ryl/testXMLHttpRequest1.html and it looks like
intervals do the same thing somehow.
Assignee: darin → bzbarsky
Priority: -- → P1
Summary: XmlHttpRequest leaks memory (all) and eventually crashes (OS X and Linux) → [FIX]XmlHttpRequest leaks memory (all) and eventually crashes (OS X and Linux)
Target Milestone: --- → mozilla1.8beta2
Comment on attachment 183800 [details] [diff] [review]
Patch

a=brendan.

/be
Attachment #183800 - Flags: approval1.8b2? → approval1.8b2+
Fix checked in.

William, if you get a chance to test a nightly build, could you verify whether
this patch fixed the issue you were seeing?
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
I have verified the fix, build 2005052009, by running against the original AJAX applications that were 
problematic. Rock solid over a two day stress test!
Excellent!  Marking verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: