Last Comment Bug 295074 - POST responses remain in the memory cache when using XMLHttpRequest => huge memory leak
: POST responses remain in the memory cache when using XMLHttpRequest => huge m...
Status: RESOLVED FIXED
: fixed1.8
Product: Core
Classification: Components
Component: Networking (show other bugs)
: Trunk
: x86 All
: -- major (vote)
: mozilla1.8beta5
Assigned To: Darin Fisher
:
Mentors:
Depends on: 308587
Blocks:
  Show dependency treegraph
 
Reported: 2005-05-21 16:33 PDT by Sascha Schumann
Modified: 2006-03-12 18:34 PST (History)
8 users (show)
mscott: blocking1.8b5+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 patch (1.26 KB, patch)
2005-05-21 17:52 PDT, Darin Fisher
cbiesinger: review+
jst: superreview+
asa: approval1.8b4+
Details | Diff | Review
v2 patch (4.71 KB, patch)
2005-09-19 16:24 PDT, Darin Fisher
cbiesinger: review+
jst: superreview+
asa: approval1.8b5+
Details | Diff | Review

Description Sascha Schumann 2005-05-21 16:33:26 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.8) Gecko/20050511
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b2) Gecko/20050522 Firefox/1.0+

When XMLHttpRequest sends a POST request, the complete response remains in the
memory cache. 

This becomes a huge issue in XUL applications which often generate thousands of
POST requests per day.

Also closing a page does not wipe out the relevant memory cache entries.

Mozilla also ignores anti-caching headers (I am not sure whether they should
affect the memory cache).

Expires: Thu, 19 Nov 1981 08:52:00 GMT
Cache-Control: no-store, no-cache, must-revalidate, post-check=0, pre-check=0
Pragma: no-cache

The behaviour can be observed in XMLHttpRequest's sync and async mode.

Compressing/gzipping the output reduces the impact of this issue, as the memory
cache apparently contains the literal data sent by the server. 

Reproducible: Always

Steps to Reproduce:
0. Start a fresh Mozilla
1. Go to http://chatserv.de/xmlhttprequest.html
2. Wait until the counter reaches 299. An alert will pop up.
3. Open "about:cache"

Actual Results:  
Memory usage increases by at least 300 * 17KB = 5MB.

Expected Results:  
POST responses are NOT cachable, especially if the server tells you not to cache. 

Mozilla must not cache the responses or at least free them as early as possible.
Comment 1 Darin Fisher 2005-05-21 16:55:25 PDT
Have you looked at "about:cache" when this happens?  Is the memory cache
exceeding its size limits?  This may be working as designed.
Comment 2 Sascha Schumann 2005-05-21 17:15:07 PDT
The memory cache is not exceeding its size limits.

My main point:

XMLHttpRequest should not pollute the memory cache with uncachable responses.

Christian Biesinger mentioned that caching _all_ responses might be necessary
for view source/save page.

That is certainly true in the browser context. It does not explain though why
XMLHttpRequest should waste resources with responses who will never ever be used
again. I hope that is not by design.
Comment 3 Darin Fisher 2005-05-21 17:45:32 PDT
> That is certainly true in the browser context. It does not explain though why
> XMLHttpRequest should waste resources with responses who will never ever be used
> again. I hope that is not by design.

Agreed.  I don't believe the authors of nsXMLHttpRequest intended this behavior.
 I suspect they were unaware of the issue.

My comments (about design) were in regard to the default caching behavior of the
networking library on which nsXMLHttpRequest is implemented.

A fix is probably trivial to implement.  We should simply add the
INHIBIT_CACHING load flag to the channel when issuing a POST request via
nsXMLHttpRequest.
Comment 4 Darin Fisher 2005-05-21 17:52:32 PDT
Created attachment 184211 [details] [diff] [review]
v1 patch
Comment 5 Christian :Biesinger (don't email me, ping me on IRC) 2005-05-21 18:05:30 PDT
Comment on attachment 184211 [details] [diff] [review]
v1 patch

(if anyone wants to consider this for any branches, they should note bug 289374
(and bug 232385))
Comment 6 Sascha Schumann 2005-05-21 18:06:21 PDT
v1 patch tested + confirmed: There are no responses in either cache from
XMLHttpRequest. Memory usage remains stable throughout the test run of
http://chatserv.de/xmlhttprequest.html .

Comment 7 Johnny Stenback (:jst, jst@mozilla.com) 2005-06-09 16:12:00 PDT
Comment on attachment 184211 [details] [diff] [review]
v1 patch

sr=jst
Comment 8 Boris Zbarsky [:bz] (Out June 25-July 6) 2005-07-27 23:07:04 PDT
Nominating for blocking so we don't forget to land this ready-to-go patch.
Comment 9 Darin Fisher 2005-07-28 15:24:39 PDT
fixed-on-trunk
Comment 10 justin hugh daly 2005-09-08 14:04:41 PDT
is this supposed to be fixed in the firefox beta?

when i launch the test url: http://chatserv.de/xmlhttprequest.html

and view the stats on about:cache and about:cache?device=memory i still see
memory increases and entries for chatserv

can anyone confirm this?
Comment 11 Darin Fisher 2005-09-08 16:05:54 PDT
Interesting.  I see that problem too.  It seems to have avoided the disk cache,
but it still populates the memory cache.  That's definitely not what was desired.
Comment 12 Darin Fisher 2005-09-08 16:07:25 PDT
reopening.  this isn't fixed.
Comment 13 Darin Fisher 2005-09-12 12:48:30 PDT
What's odd about this bug is that I cannot reproduce the problem using a debug
build.  In my debug build, the memory cache entries do not exist, but in my
release build, I do see the objects in the memory cache.  This is very strange.
Comment 14 Scott MacGregor 2005-09-13 10:12:46 PDT
Sounds like Darin is asctively looking into this and the leak sounds bad to me.
But we'll re-evaluate the nomination once we have a fix to do risk analysis on.
Comment 15 Darin Fisher 2005-09-14 18:24:07 PDT
WOW.  Ok, I don't know how to explain this yet, but it appears that I can
reproduce this bug if I manually set browser.cache.memory.capacity to some fixed
value.  If I set it to -1 or if I remove the pref (which is the default
configuration), then this bug does not occur.  That pref only controls the size
of the memory cache, it does not disable the memory cache, so I don't know why
it would impact this bug.  Investigating...
Comment 16 Darin Fisher 2005-09-14 18:48:03 PDT
OK.  More info:  It turns out that if you have more than 2GB of RAM on your
system (Linux), then your memory cache size will be zero!  This is due to bug
308587.  If your memory cache size is zero, then this bug will appear to be fixed.

This explains why I observed an effect by setting browser.cache.memory.capacity
manually in my prefs.

So, the net take-away is that this bug is indeed still here.  So, I will try to
figure out why the cache entries are not being cleared.
Comment 17 Darin Fisher 2005-09-19 14:28:03 PDT
OK, the problem observed with the testcase is caused by the fact that null is
passed to .send().  As a result, we do not have a postDataStream, and therefore
we do not run through the code that calls SetUploadStream.  I think we need to
trigger the loadFlag munging code when the method is POST and not just when the
postDataStream is non-null.
Comment 18 Darin Fisher 2005-09-19 16:24:59 PDT
Created attachment 196717 [details] [diff] [review]
v2 patch

This patch does the trick.  The first set of changes just eliminates a compile
warning under GCC 3.2.
Comment 19 Christian :Biesinger (don't email me, ping me on IRC) 2005-09-21 14:22:28 PDT
Comment on attachment 196717 [details] [diff] [review]
v2 patch

r=biesi
Comment 20 Johnny Stenback (:jst, jst@mozilla.com) 2005-09-22 11:30:55 PDT
Comment on attachment 196717 [details] [diff] [review]
v2 patch

sr=jst
Comment 21 Darin Fisher 2005-09-22 11:32:39 PDT
fixed-on-trunk
Comment 22 Tracy Walker [:tracy] 2005-09-23 11:33:17 PDT
I can verify this on Firefox trunk nightly builds from 09-23 Wndows and Linux. 
No response.txt is getting cached in memory or disc. But what is puzzling me is
that Mac 1.5 branch build from 9-23 also didn't show any response.txt in cache.
I verified I am correctly understanding the testcase with a test on Moz 1.7.12.
Note that I did not see the status count to 300 on the Firefox builds as it did
on Moz 1.7.12.  The only 2 Firefox cache entries are for an empty favicon and
the html page itself.

Please verify.
Comment 23 Darin Fisher 2005-09-23 11:53:48 PDT
fixed1.8
Comment 24 Worcester12345 2005-09-23 15:09:26 PDT
mlk keyword missing (for historical purposes)
Comment 25 Christian :Biesinger (don't email me, ping me on IRC) 2005-09-23 16:07:33 PDT
er, this is not actually a leak, is it?
Comment 26 Darin Fisher 2005-09-23 16:08:43 PDT
(In reply to comment #25)
> er, this is not actually a leak, is it?

Technically, yeah.. you are correct.

Note You need to log in before you can comment on or make changes to this bug.