935595 - NS_NOTREACHED("unexpected request") @ nsHttpChannel::OnStopRequest on partially cached download

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Honza Bambas (:mayhemer)]

Attachment: v1

STR:
- play a flash video which is < 5MB in length, resumable and veeery slow to download *)
- interrupt the load sooner then we reach the 5MB
- open link to the video source in a new tab (e.g. by grabbing the URL via the web console) with intention to download it
- start the download (press enter :))
=> NOT_REACHED soon

The problem is here:
- we cache partially the video file (say we have ~3MB of the data) by viewing just part of it
- the download channel will make an If-Range request for the rest, will get 206
- in OnStartRequest it will doom and throw the entry + the cache pump away (bug 55307)
- in OnStopRequest from the cache pump (when reading the partially cached content is done) we check whether the request is sane, it is, but mCachePump (== request) is null

*) if larger, the cache entry will be doomed very soon and there will be no fun


Solution:
Don't close the cache entry + pump when we are processing a partial response.  I tried to just keep the cache pump, but later we crashed on accessing the entry which was also nullified.

Comment 1

[Honza Bambas (:mayhemer)]

Attachment: v2

- shows up that the concurrent read/write access is the same case, we simply need the entry at OnStopRequest to make decisions

I personally play with an idea to remove the CloseCacheEntry() line here at all.  At least when the new cache is enabled?

Comment 2

[Honza Bambas (:mayhemer)]

Actually blocks bug 922671 from working 100% correctly.

Comment 3

[Honza Bambas (:mayhemer)]

Comment on attachment 828133 [details] [diff] [review]
v2

https://hg.mozilla.org/integration/mozilla-inbound/rev/4fbc00f0d464
https://tbpl.mozilla.org/?tree=Try&rev=69d73cf7f79a

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4fbc00f0d464

Comment 5

[Ada [:adalucinet]]

(In reply to Honza Bambas (:mayhemer) from comment #0)
> STR:
> - play a flash video which is < 5MB in length, resumable and veeery slow to
> download *)
> - interrupt the load sooner then we reach the 5MB
> - open link to the video source in a new tab (e.g. by grabbing the URL via
> the web console) with intention to download it
> - start the download (press enter :))
> => NOT_REACHED soon

I couldn't reproduce with Nightly from 2013-11-07 on Ubuntu 13.10 32bit machine. Tried with several small flash videos on youtube.
Could you please point to an URL in order to reproduce and verify this issue?

Comment 6

[Honza Bambas (:mayhemer)]

I absolutely don't remember where I've spot this.  Probably some funny videos or free porn site or something like that :)  Also let you know that bug 913820 hides the problem.  That bug reintroduced the cache entry size limit.  So, when the video file is larger then 5MB it's not cached.  This bug then cannot happen.  But I think this is also about a great piece of luck to hit.  Note that you need a debug build to let NOTREACHED to do something.

Comment 7

[Ada [:adalucinet]]

(In reply to Honza Bambas (:mayhemer) from comment #6)
> Note that you need a debug build to let NOTREACHED to do
> something.

Unfortunately, this issue is not visible for me nor with a Nightly debug build. Dropping verifyme keyword since the QA couldn't reproduce this issue.