POST responses remain in the memory cache when using XMLHttpRequest => huge memory leak

RESOLVED FIXED in mozilla1.8beta5

Status

()

Core
Networking
--
major
RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: Sascha Schumann, Assigned: Darin Fisher)

Tracking

({fixed1.8})

Trunk
mozilla1.8beta5
x86
All
fixed1.8
Points:
---
Bug Flags:
blocking1.8b5 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

12 years ago
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.
Assignee: nobody → darin
Component: General → Networking: Cache
Product: Firefox → Core
QA Contact: general → networking.cache
Version: unspecified → Trunk
(Assignee)

Comment 1

12 years ago
Have you looked at "about:cache" when this happens?  Is the memory cache
exceeding its size limits?  This may be working as designed.
(Reporter)

Comment 2

12 years ago
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.
(Assignee)

Comment 3

12 years ago
> 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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: helpwanted
(Assignee)

Updated

12 years ago
Status: NEW → ASSIGNED
Component: Networking: Cache → Networking
Target Milestone: --- → mozilla1.8beta3
(Assignee)

Comment 4

12 years ago
Created attachment 184211 [details] [diff] [review]
v1 patch
Attachment #184211 - Flags: superreview?(jst)
Attachment #184211 - Flags: review?(cbiesinger)
(Assignee)

Updated

12 years ago
Keywords: helpwanted
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))
Attachment #184211 - Flags: review?(cbiesinger) → review+
(Reporter)

Comment 6

12 years ago
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 on attachment 184211 [details] [diff] [review]
v1 patch

sr=jst
Attachment #184211 - Flags: superreview?(jst) → superreview+
(Assignee)

Updated

12 years ago
Attachment #184211 - Flags: approval1.8b4?
Attachment #184211 - Flags: approval1.8b3?

Updated

12 years ago
Attachment #184211 - Flags: approval1.8b4?
Attachment #184211 - Flags: approval1.8b4+
Attachment #184211 - Flags: approval1.8b3?
Nominating for blocking so we don't forget to land this ready-to-go patch.
Flags: blocking1.8b4?
(Assignee)

Comment 9

12 years ago
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Updated

12 years ago
Flags: blocking1.8b4?

Comment 10

12 years ago
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?
(Assignee)

Comment 11

12 years ago
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.
(Assignee)

Comment 12

12 years ago
reopening.  this isn't fixed.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Updated

12 years ago
Severity: normal → major
Status: REOPENED → ASSIGNED
Flags: blocking1.8b5?
Target Milestone: mozilla1.8beta3 → mozilla1.8beta4
(Assignee)

Comment 13

12 years ago
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

12 years ago
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.
Flags: blocking1.8b5? → blocking1.8b5+
(Assignee)

Comment 15

12 years ago
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...
(Assignee)

Comment 16

12 years ago
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.
Depends on: 308587
(Assignee)

Comment 17

12 years ago
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.
(Assignee)

Comment 18

12 years ago
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.
Attachment #184211 - Attachment is obsolete: true
Attachment #196717 - Flags: superreview?(jst)
Attachment #196717 - Flags: review?(cbiesinger)
(Assignee)

Updated

12 years ago
Target Milestone: mozilla1.8beta4 → mozilla1.8beta5
Comment on attachment 196717 [details] [diff] [review]
v2 patch

r=biesi
Attachment #196717 - Flags: review?(cbiesinger) → review+
Comment on attachment 196717 [details] [diff] [review]
v2 patch

sr=jst
Attachment #196717 - Flags: superreview?(jst) → superreview+
(Assignee)

Comment 21

12 years ago
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago12 years ago
Resolution: --- → FIXED
(Assignee)

Updated

12 years ago
Attachment #196717 - Flags: approval1.8b5?

Updated

12 years ago
Attachment #196717 - Flags: approval1.8b5? → approval1.8b5+
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.
(Assignee)

Comment 23

12 years ago
fixed1.8
Keywords: fixed1.8

Comment 24

12 years ago
mlk keyword missing (for historical purposes)
(Assignee)

Updated

12 years ago
Keywords: mlk
er, this is not actually a leak, is it?
(Assignee)

Comment 26

12 years ago
(In reply to comment #25)
> er, this is not actually a leak, is it?

Technically, yeah.. you are correct.
Keywords: mlk
You need to log in before you can comment on or make changes to this bug.