Persona is no longer an option for authentication on BMO. For more details see Persona Deprecated.
Last Comment Bug 715837 - Crash @ mozilla::net::HttpChannelParent::RecvMarkOfflineCacheEntryAsForeign
: Crash @ mozilla::net::HttpChannelParent::RecvMarkOfflineCacheEntryAsForeign
: crash, regression, topcrash
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: 9 Branch
: ARM Android
: -- critical (vote)
: mozilla15
Assigned To: Honza Bambas (:mayhemer)
: Patrick McManus [:mcmanus]
Depends on:
  Show dependency treegraph
Reported: 2012-01-06 01:02 PST by Scoobidiver (away)
Modified: 2012-05-08 04:49 PDT (History)
4 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

v1 (6.76 KB, patch)
2012-05-03 10:36 PDT, Honza Bambas (:mayhemer)
jduell.mcbugs: review+
Details | Diff | Splinter Review
v1.1 (as landed) (6.92 KB, patch)
2012-05-07 13:02 PDT, Honza Bambas (:mayhemer)
honzab.moz: review+
honzab.moz: checkin+
Details | Diff | Splinter Review

Description Scoobidiver (away) 2012-01-06 01:02:21 PST
It's #5 top crasher in Fennec 9.0.

Signature 	mozilla::net::HttpChannelParent::RecvMarkOfflineCacheEntryAsForeign More Reports Search
UUID	5c4b82ed-1f85-4823-b897-a07b52120104
Date Processed	2012-01-04 15:38:12.278705
Uptime	183
Last Crash	more than 3 months before submission
Install Age	1.2 weeks since version was first installed.
Install Time	2011-12-27 19:49:06
Product	Fennec
Version	9.0
Build ID	20111216135618
Release Channel	release
OS	Linux
OS Version	0.0.0 Linux #1 PREEMPT Sat Aug 20 10:40:49 IST 2011 armv7l
Build Architecture	arm
Build Architecture Info	
Crash Reason	SIGSEGV
Crash Address	0x0
App Notes 	
motorola DROIDX

Frame 	Module 	Signature [Expand] 	Source
0 	mozilla::net::HttpChannelParent::RecvMarkOfflineCacheEntryAsForeign 	netwerk/protocol/http/HttpChannelParent.cpp:365
1 	mozilla::net::PHttpChannelParent::OnMessageReceived 	obj-firefox/ipc/ipdl/PHttpChannelParent.cpp:659
2 	mozilla::dom::PContentParent::OnMessageReceived 	obj-firefox/ipc/ipdl/PContentParent.cpp:752
3 	mozilla::ipc::AsyncChannel::OnDispatchMessage 	ipc/glue/AsyncChannel.cpp:294
4 	mozilla::ipc::RPCChannel::OnMaybeDequeueOne 	ipc/glue/RPCChannel.cpp:433
5 	RunnableMethod<mozilla::ipc::RPCChannel, bool , Tuple0>::Run 	ipc/chromium/src/base/tuple.h:383
6 	mozilla::ipc::RPCChannel::DequeueTask::Run 	RPCChannel.h:464
7 	MessageLoop::RunTask 	ipc/chromium/src/base/
8 	MessageLoop::DeferOrRunPendingTask 	ipc/chromium/src/base/
9 	MessageLoop::DoWork 	ipc/chromium/src/base/
10 	mozilla::ipc::DoWorkRunnable::Run 	ipc/glue/MessagePump.cpp:70
11 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:631
12 	NS_ProcessNextEvent_P 	obj-firefox/xpcom/build/nsThreadUtils.cpp:245
13 	mozilla::ipc::MessagePump::Run 	ipc/glue/MessagePump.cpp:110
14 	MessageLoop::RunInternal 	ipc/chromium/src/base/
15 	MessageLoop::Run 	ipc/chromium/src/base/
16 	nsBaseAppShell::Run 	widget/src/xpwidgets/nsBaseAppShell.cpp:189
17 	nsAppStartup::Run 	toolkit/components/startup/nsAppStartup.cpp:228
Comment 1 Honza Bambas (:mayhemer) 2012-02-03 09:40:32 PST
This can happen only for a top-level document loading channels being satisfied from offline cache.  mChannel had already been nullified from the parent channel.

Marking a cache entry as foreign is triggered from content sink (early) during parsing of the document content on the content process.  It prevents load of the document from the offline cache on next load.  The document is reloaded after that marking.

Problem is that we will simply loop when the cache entry is not marked as foreign because on the next load attempt the conditions leading to mark/reload action will pass again.

We cannot hold the cache entry or the channel on the chrome process to not hold on loads of the same page in other tabs.  We artificially throw the reference to the channel and the cache entry away after the document had been delivered to the content process.

We could delay this artificial release until/after "SetMark" happened to be called when loading a top level doc from offline cache.
Comment 2 Scoobidiver (away) 2012-03-28 04:36:35 PDT
It's still #5 top browser crasher in XUL Fennec 10.0.3esr (340,000 ADU).
The Aurora (800 ADU) and Nightly (500 ADU) population seems unaffected in Native Fennec.
Comment 3 Honza Bambas (:mayhemer) 2012-05-03 10:36:03 PDT
Created attachment 620769 [details] [diff] [review]

- simple patch
- leaves the cleanup logic intact
- to mark an entry offline HttpChannelParent is using a class, provided by nsHttpChannel, having a method MarkForeign()
- the class just keeps what's necessary to perform the op: app cache instance and the key
Comment 4 Jason Duell [:jduell] (needinfo me) 2012-05-04 13:55:40 PDT
Comment on attachment 620769 [details] [diff] [review]

Review of attachment 620769 [details] [diff] [review]:

Looks good.

> The Aurora (800 ADU) and Nightly (500 ADU) population seems unaffected in Native Fennec.

Yes, because they don't use e10s.  We'll need this for B2G, but don't need to backport anywhere.

::: netwerk/protocol/http/nsHttpChannel.h
@@ +160,5 @@
>          mRequestHead.SetHeader(nsHttp::Referer, spec);
>          return NS_OK;
>      }
> +    class OfflineCacheEntryAsForeignMarker {

Add comment:

// This allows cache entry to be marked as foreign even after channel itself
// is gone.  Needed for e10s (see HttpChannelParent::RecvDocumentChannelCleanup)
Comment 6 Geoff Lankow (:darktrojan) 2012-05-07 04:46:43 PDT
Backed out, it's burning all the platforms.
Comment 7 Honza Bambas (:mayhemer) 2012-05-07 04:53:31 PDT
(In reply to Geoff Lankow (:darktrojan) from comment #6)
> Backed out, it's burning all the platforms.

Except Windows I assume, since it is too much benevolent...  Thanks for the backout.
Comment 8 Honza Bambas (:mayhemer) 2012-05-07 13:02:22 PDT
Created attachment 621701 [details] [diff] [review]
v1.1 (as landed)
Comment 9 Ryan VanderMeulen [:RyanVM] 2012-05-07 17:51:31 PDT
Original landing and backout:

Also, should this have a test?
Comment 10 Ed Morley (Away 28th Oct -> 6th Nov) [:emorley] 2012-05-08 03:13:01 PDT
Comment 11 Honza Bambas (:mayhemer) 2012-05-08 04:35:15 PDT
(In reply to Ryan VanderMeulen from comment #9)
> Also, should this have a test?

There is a test.  Just don't run on e10s (fennec).  I'm able to make fennec crash w/o this fix quit easily.

Note You need to log in before you can comment on or make changes to this bug.