Closed Bug 1048348 Opened 10 years ago Closed 10 years ago

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

Categories

(Core :: Networking: HTTP, defect)

32 Branch
x86
Windows NT
defect
Not set
critical

Tracking

()

RESOLVED DUPLICATE of bug 1066726

People

(Reporter: lizzard, Assigned: mayhemer)

Details

(Keywords: crash, topcrash)

Crash Data

Attachments

(1 file)

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
michal - want to take a look?
Flags: needinfo?(michal.novotny)
Assignee: nobody → michal.novotny
Flags: needinfo?(michal.novotny)
Michal, if you want I can steal this from you.
Assignee: michal.novotny → honzab.moz
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.
Attached patch v1Splinter Review
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.
Attachment #8483682 - Flags: review?(jduell.mcbugs)
Status: NEW → ASSIGNED
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?
Attachment #8483682 - Flags: review?(jduell.mcbugs) → review+
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: