1289163 - Number of empty entries created in the HTTP cache, for unexpected schemes.

Status: RESOLVED FIXED

(Core :: Networking: Cache, defect)

Description

[Honza Bambas (:mayhemer)]

Nightly (up to date), working profile (way old), windows.

I have a cache (~240MB of the 350MB limit) having a huge (one third) of entries (~3000) being completely empty (=no data and no metadata, just URL + control sums).

I can see cache entries created also for data: (usually images) and also even for about:blank.  All are just those empty entries that should never exist in the cache.

This might be coming from net predictor, that is probably using the API correctly, but the cache does something wrong then.

Comment 1

[u408661]

Just a data point - predictor shouldn't be doing *anything* for non-http{,s} schemes, let alone opening/creating cache entries for them. If it is, that's something separate we should investigate.

Comment 2

[Honza Bambas (:mayhemer)]

(In reply to Nicholas Hurley [:nwgh][:hurley] from comment #1)
> Just a data point - predictor shouldn't be doing *anything* for non-http{,s}
> schemes, let alone opening/creating cache entries for them. If it is, that's
> something separate we should investigate.

Thanks!  Good to know.

Comment 3

[Honza Bambas (:mayhemer)]

(In reply to Nicholas Hurley [:nwgh][:hurley] from comment #1)
> Just a data point - predictor shouldn't be doing *anything* for non-http{,s}
> schemes, let alone opening/creating cache entries for them. If it is, that's
> something separate we should investigate.

So...

 	xul.dll!mozilla::net::CacheStorage::AsyncOpenURI(0x1dcd0fa0, {...}, 0x00000032, 0x218466a0) Line 79	C++
 	xul.dll!mozilla::net::Predictor::UpdateCacheabilityInternal(0x1dcd0fa0, 0x1ecb0600, 0x000000c8, {...}) Line 2459	C++
 	xul.dll!mozilla::net::Predictor::UpdateCacheability(0x1dcd0fa0, 0x1ecb0600, 0x000000c8, {...}, 0x143c75e0, 0x218451c0) Line 2441	C++
>	xul.dll!mozilla::net::nsHttpChannel::ProcessResponse() Line 1798	C++
 	xul.dll!mozilla::net::nsHttpChannel::OnStartRequest(0x1f016580, 0x00000000) Line 6225	C++
 	xul.dll!nsInputStreamPump::OnStateStart() Line 524	C++
 	xul.dll!nsInputStreamPump::OnInputStreamReady(0x20262160) Line 426	C++

Where sourceURI == "about:blank", because the http channel's referrer is "about:blank".

Obtained:

    nsCOMPtr<nsIURI> referrer = GetReferringPage();
    if (!referrer) {
        referrer = mReferrer;
    }
    if (referrer) {
        nsCOMPtr<nsILoadContextInfo> lci = GetLoadContextInfo(this);
        mozilla::net::Predictor::UpdateCacheability(referrer, mURI, httpStatus,
                                                    mRequestHead, mResponseHead,
                                                    lci);
    }

mReferrer is null.

Nick, can you do anything about this?

Michal, would it be easy (better said - trivial) enough to don't allow such entries be flushed to the cache actually?  I first would like to prevent their existence at all (hence handing to Nick), because for the cache to decide whether or how long to keep an empty entry around may be hard.

Comment 4

[u408661]

(In reply to Honza Bambas (:mayhemer) from comment #3)
> Nick, can you do anything about this?

Yep, looks like I just forgot to put the http{,s}-only checks in UpdateCacheability. I'll prep the patch once my current local build completes.

Comment 5

[Honza Bambas (:mayhemer)]

Ups... had to look at the open flags first!  Yes, it's opened as READONLY, so no file should ever be created for this.  And it isn't!

I'm looking further and it seems that the report of empty entries is coming just from the about:cache.  Those are temporarily kept in memory (and in the index) and SHOULD never go to the disk.  But do.  Why?  When you click an entry in about:cache, it's open WITH the READONLY flag, but even just that makes the entry be (somehow) marked dirty and written to disk.


I'll reassign to Michal, since file handling in the cache is his area more than mine.

The goal here would be to simply never write anything, until the entry is written at least something (meta or data).

I'll make this blocking bug 1289164 since that one could hide this issue.

(In reply to Nicholas Hurley [:nwgh][:hurley] from comment #4)
> (In reply to Honza Bambas (:mayhemer) from comment #3)
> > Nick, can you do anything about this?
> 
> Yep, looks like I just forgot to put the http{,s}-only checks in
> UpdateCacheability. I'll prep the patch once my current local build
> completes.

I don't think there is much for you to do here actually, Nick.  If you filter this just by http(s), you may still create 'empty' entries that will get reported in about:cache and may get to the disk.

Comment 6

[u408661]

(In reply to Honza Bambas (:mayhemer) from comment #5)
> I don't think there is much for you to do here actually, Nick.  If you
> filter this just by http(s), you may still create 'empty' entries that will
> get reported in about:cache and may get to the disk.

Maybe not on this bug, but it does make sense for me to make the change mentioned anyway - no use in doing work/attempting to open cache entries that we'll never make use of! I'll file a new bug for that one, though.

Comment 7

[Honza Bambas (:mayhemer)]

(In reply to Nicholas Hurley [:nwgh][:hurley] from comment #6)
> Maybe not on this bug, but it does make sense for me to make the change

Bug 1289564.

Comment 8

[Honza Bambas (:mayhemer)]

The real problem actually is not in the file back end but on the cache entry level.  We touch the cache file state (mFile->OnFetched) even when an entry has not been found.  That makes it write itself to the disk.

Comment 9

[Honza Bambas (:mayhemer)]

Attachment: v1 (CacheFile->OnFetched() called only on actual fetch)

- call mFile->OnFetched() only when we actually deliver an entry
- it was happening also when a readonly open was performed and there were no entry (no file)
- it marked the metadata as dirty and those were written (perfectly legal, we are deliberately dumb at this, upper layers take care to not write empty entries - now)

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a66acdcf5646
Make sure no HTTP cache entry file is created when URL opened as read-only. r=michal

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a66acdcf5646