1048348 - crash in mozilla::net::nsHttpChannel::OnDoneReadingPartialCacheEntry(bool*)

Status: RESOLVED DUPLICATE of bug 1066726

(Core :: Networking: HTTP, defect)

Description

[Liz Henry (:lizzard) (relman/hg->git project)]

This bug was filed from the Socorro interface and is 
report bp-473ec10c-a66b-4eb8-99ec-87a0d2140803.
=============================================================

mozilla::net::nsHttpChannel::OnDoneReadingPartialCacheEntry(bool*) is the #15 topcrasher for Firefox 34.0a1 with 76/14787 crashes in the last 7 days. It appears to often, but not always, be a startup crash. Common urls reported for the crash are about:home and about:newtab. 

crashing thread: 

0 	xul.dll 	mozilla::net::nsHttpChannel::OnDoneReadingPartialCacheEntry(bool*) 	netwerk/protocol/http/nsHttpChannel.cpp
1 	xul.dll 	mozilla::net::nsHttpChannel::OnStopRequest(nsIRequest*, nsISupports*, tag_nsresult) 	netwerk/protocol/http/nsHttpChannel.cpp
2 	xul.dll 	nsInputStreamPump::OnStateStop() 	netwerk/base/src/nsInputStreamPump.cpp
3 	xul.dll 	nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) 	netwerk/base/src/nsInputStreamPump.cpp
4 	xul.dll 	nsInputStreamReadyEvent::Run() 	xpcom/io/nsStreamUtils.cpp
5 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp
6 	xul.dll 	NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/glue/nsThreadUtils.cpp
7 	xul.dll 	mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp
8 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc
9 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc
10 	xul.dll 	nsBaseAppShell::Run() 	widget/xpwidgets/nsBaseAppShell.cpp
11 	xul.dll 	nsAppShell::Run() 	widget/windows/nsAppShell.cpp
12 	xul.dll 	nsAppStartup::Run() 	toolkit/components/startup/nsAppStartup.cpp
13 	xul.dll 	XREMain::XRE_mainRun() 	toolkit/xre/nsAppRunner.cpp
14 	xul.dll 	XREMain::XRE_main(int, char** const, nsXREAppData const*) 	toolkit/xre/nsAppRunner.cpp
15 	xul.dll 	XRE_main 	toolkit/xre/nsAppRunner.cpp
16 	firefox.exe 	do_main 	browser/app/nsBrowserApp.cpp
17 	firefox.exe 	NS_internal_main(int, char**) 	browser/app/nsBrowserApp.cpp
18 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp
19 	firefox.exe 	__tmainCRTStartup 	f:/dd/vctools/crt_bld/self_x86/crt/src/crtexe.c:552
20 	kernel32.dll 	BaseThreadInitThunk 	
21 	ntdll.dll 	__RtlUserThreadStart 	
22 	ntdll.dll 	_RtlUserThreadStart

Comment 1

[Patrick McManus [:mcmanus]]

michal - want to take a look?

Comment 2

[Honza Bambas (:mayhemer)]

Michal, if you want I can steal this from you.

Comment 3

[Honza Bambas (:mayhemer)]

One idea is that we throw the entry away in nsHttpChannel::CallOnStartRequest() when mCacheEntry->SetPredictedDataSize() fails.  I'll try to write a test to confirm/disprove it.

Comment 4

[Honza Bambas (:mayhemer)]

Attachment: v1

So, I was able to confirm the crash with a test (included).  The thing is that when smart size changes (shrinks) request for partial content may hit the single entry size limit.  We throw the entry away, but the code later is not willing to live with it.

This seems to me realistic, also because cache v1 SetPredictedDataSize was always returning NS_OK and this started with Fx 32.

Comment 5

[Jason Duell]

Comment on attachment 8483682 [details] [diff] [review]
v1

Review of attachment 8483682 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/protocol/http/nsHttpChannel.cpp
@@ +895,5 @@
>      if (mResponseHead && mResponseHead->ContentCharset().IsEmpty())
>          mResponseHead->SetContentCharset(mContentCharsetHint);
>  
>      if (mResponseHead && mCacheEntry) {
>          // If we have a cache entry, set its predicted size to ContentLength to

worth fixing up comment here?

::: netwerk/test/unit/test_partial_response_too_big_cache_entry.js
@@ +64,5 @@
> +}
> +
> +function firstTimeThrough(request, buffer)
> +{
> +dump(buffer);

remove dump before landing?