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

RESOLVED DUPLICATE of bug 1066726

Status

()

--
critical
RESOLVED DUPLICATE of bug 1066726
4 years ago
4 years ago

People

(Reporter: lizzard, Assigned: mayhemer)

Tracking

({crash, topcrash})

32 Branch
x86
Windows NT
crash, topcrash
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(1 attachment)

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)
(Assignee)

Comment 2

4 years ago
Michal, if you want I can steal this from you.
Assignee: michal.novotny → honzab.moz
(Assignee)

Comment 3

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

Comment 4

4 years ago
Created attachment 8483682 [details] [diff] [review]
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.
Attachment #8483682 - Flags: review?(jduell.mcbugs)
(Assignee)

Updated

4 years ago
Status: NEW → ASSIGNED

Comment 5

4 years ago
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+
(Assignee)

Updated

4 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1066726
You need to log in before you can comment on or make changes to this bug.