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)
Core
Networking
Tracking
()
VERIFIED
FIXED
mozilla1.8beta2
People
(Reporter: bill, Assigned: bzbarsky)
Details
(Keywords: memory-leak, testcase)
Attachments
(2 files)
1.13 KB,
text/html
|
Details | |
937 bytes,
patch
|
darin.moz
:
review+
darin.moz
:
superreview+
brendan
:
approval1.8b2+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•19 years ago
|
||
Comment 2•19 years ago
|
||
This looks like bug 280808. Reporter, are you sure that the bug also exists in the trunk-version ?
Reporter | ||
Comment 3•19 years ago
|
||
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.
Assignee | ||
Comment 4•19 years ago
|
||
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
Assignee | ||
Comment 5•19 years ago
|
||
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.
Assignee | ||
Comment 6•19 years ago
|
||
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)
Comment 7•19 years ago
|
||
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.
Updated•19 years ago
|
Attachment #183800 -
Flags: superreview?(darin)
Attachment #183800 -
Flags: superreview+
Attachment #183800 -
Flags: review?(darin)
Attachment #183800 -
Flags: review+
Assignee | ||
Comment 8•19 years ago
|
||
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?
Assignee | ||
Comment 9•19 years ago
|
||
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 10•19 years ago
|
||
Comment on attachment 183800 [details] [diff] [review] Patch a=brendan. /be
Attachment #183800 -
Flags: approval1.8b2? → approval1.8b2+
Assignee | ||
Comment 11•19 years ago
|
||
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
Reporter | ||
Comment 12•19 years ago
|
||
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!
You need to log in
before you can comment on or make changes to this bug.
Description
•