Last Comment Bug 760955 - crash in mozilla::net::nsHttpChannel::InstallOfflineCacheListener
: crash in mozilla::net::nsHttpChannel::InstallOfflineCacheListener
Status: VERIFIED FIXED
[native-crash],[fixed in b2g],[qa+]
: crash, regression, reproducible, topcrash
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: 15 Branch
: All All
: -- critical (vote)
: mozilla16
Assigned To: Brian Smith (:briansmith, :bsmith, use NEEDINFO?)
:
Mentors:
: 763088 766224 769210 769211 (view as bug list)
Depends on:
Blocks: twitter.com 722034 746018 767208
  Show dependency treegraph
 
Reported: 2012-06-03 02:44 PDT by Scoobidiver (away)
Modified: 2013-10-18 16:21 PDT (History)
18 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
unaffected
unaffected
unaffected
+
fixed


Attachments
Fix null pointer deref in InstallOfflineCacheEntry (5.81 KB, patch)
2012-06-03 18:19 PDT, Brian Smith (:briansmith, :bsmith, use NEEDINFO?)
hurley: review+
lukasblakk+bugs: approval‑mozilla‑aurora-
lukasblakk+bugs: approval‑mozilla‑beta-
Details | Diff | Review

Description Scoobidiver (away) 2012-06-03 02:44:57 PDT
It first appeared in FennecAndroid 15.0a1/20120601. The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3aa566994890&tochange=73783bf75c4c
It's likely a regression from bug 722034 or bug 746018.

Signature 	mozilla::net::nsHttpChannel::InstallOfflineCacheListener More Reports Search
UUID	9d6c5a4b-292e-40f3-855d-9d6c22120602
Date Processed	2012-06-02 20:10:32
Uptime	202
Last Crash	4.3 minutes before submission
Install Age	5.1 hours since version was first installed.
Install Time	2012-06-02 15:03:45
Product	FennecAndroid
Version	15.0a1
Build ID	20120602030527
Release Channel	nightly
OS	Linux
OS Version	0.0.0 Linux 2.6.35.7-I9100UHKI3-CL565937 #2 SMP PREEMPT Thu Sep 8 21:17:57 KST 2011 armv7l
Build Architecture	arm
Build Architecture Info	
Crash Reason	SIGSEGV
Crash Address	0x0
App Notes 	
AdapterVendorID: smdkc210, AdapterDeviceID: GT-I9100.
AdapterDescription: 'Model: 'GT-I9100', Product: 'GT-I9100', Manufacturer: 'samsung', Hardware: 'smdkc210''.
samsung GT-I9100
samsung/GT-I9100/GT-I9100:2.3.4/GINGERBREAD/UHKI3:user/release-keys
EMCheckCompatibility	True

Frame 	Module 	Signature 	Source
0 	libxul.so 	mozilla::net::nsHttpChannel::InstallOfflineCacheListener 	netwerk/protocol/http/nsHttpChannel.cpp:3843
1 	libxul.so 	mozilla::net::nsHttpChannel::CallOnStartRequest 	netwerk/protocol/http/nsHttpChannel.cpp:975
2 	libxul.so 	mozilla::net::nsHttpChannel::ContinueProcessNormal 	netwerk/protocol/http/nsHttpChannel.cpp:1440
3 	libxul.so 	mozilla::net::nsHttpChannel::ProcessNormal 	netwerk/protocol/http/nsHttpChannel.cpp:1375
4 	libxul.so 	mozilla::net::nsHttpChannel::ProcessResponse 	netwerk/protocol/http/nsHttpChannel.cpp:1289
5 	libxul.so 	mozilla::net::nsHttpChannel::OnStartRequest 	netwerk/protocol/http/nsHttpChannel.cpp:4747
6 	libxul.so 	nsInputStreamPump::OnStateStart 	netwerk/base/src/nsInputStreamPump.cpp:416
7 	libxul.so 	nsInputStreamPump::OnInputStreamReady 	netwerk/base/src/nsInputStreamPump.cpp:367
8 	libxul.so 	nsInputStreamReadyEvent::Run 	xpcom/io/nsStreamUtils.cpp:81
9 	libxul.so 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:624
10 	libxul.so 	NS_ProcessNextEvent_P 	obj-firefox/xpcom/build/nsThreadUtils.cpp:213
11 	libxul.so 	mozilla::ipc::MessagePump::Run 	ipc/glue/MessagePump.cpp:82
12 	libxul.so 	MessageLoop::RunInternal 	ipc/chromium/src/base/message_loop.cc:208
13 	libxul.so 	MessageLoop::Run 	ipc/chromium/src/base/message_loop.cc:201
14 	libxul.so 	nsBaseAppShell::Run 	widget/xpwidgets/nsBaseAppShell.cpp:163
15 	libxul.so 	nsAppStartup::Run 	toolkit/components/startup/nsAppStartup.cpp:256
16 	libxul.so 	XREMain::XRE_mainRun 	toolkit/xre/nsAppRunner.cpp:3786
17 	libxul.so 	XREMain::XRE_main 	toolkit/xre/nsAppRunner.cpp:3863
18 	libxul.so 	XRE_main 	toolkit/xre/nsAppRunner.cpp:3939
...

More reports at:
https://crash-stats.mozilla.com/report/list?signature=mozilla%3A%3Anet%3A%3AnsHttpChannel%3A%3AInstallOfflineCacheListener
Comment 1 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2012-06-03 18:06:47 PDT
Here's the offending changeset:
http://hg.mozilla.org/mozilla-central/rev/0f3f9ff4439d#l1.10
Comment 2 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2012-06-03 18:19:09 PDT
Created attachment 629677 [details] [diff] [review]
Fix null pointer deref in InstallOfflineCacheEntry
Comment 3 Scoobidiver (away) 2012-06-08 03:06:55 PDT
It's #1 top crasher in FennecAndroid 16.0a1.
Comment 4 Aaron Train [:aaronmt] 2012-06-09 07:21:37 PDT
STR: 
i. Install Aurora or Nightly on Android
ii. https://touch.betfair.com/, allow for offline data storage via prompt
Comment 5 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2012-06-11 13:44:23 PDT
*** Bug 763088 has been marked as a duplicate of this bug. ***
Comment 6 Nicholas Hurley [:nwgh][:hurley] 2012-06-11 15:00:53 PDT
Comment on attachment 629677 [details] [diff] [review]
Fix null pointer deref in InstallOfflineCacheEntry

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

I presume (with the exception of the dump shown below) that you intended to keep the do_print statements in, to make reading the test output easier. If not, please remove them. r=me with the changes below.

::: netwerk/test/unit/test_cacheForOfflineUse_no-store.js
@@ +1,1 @@
> +u"se strict"; 

This is wrong. Perhaps it's just an artifact of how you created the patch, but please make sure this isn't committed with the double quotes.

@@ +43,5 @@
> +function asyncCheckCacheEntryExistance(entryName, shouldExist)
> +{
> +  var listener = new CacheListener();
> +  listener.onCacheEntryAvailable = function(descriptor, accessGranted, status) {
> +    dump("foo");

Get rid of this
Comment 7 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2012-06-11 15:17:05 PDT
Comment on attachment 629677 [details] [diff] [review]
Fix null pointer deref in InstallOfflineCacheEntry

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

::: netwerk/test/unit/test_cacheForOfflineUse_no-store.js
@@ +1,1 @@
> +u"se strict"; 

Nick, thank you for pointing out the typo. It should be "use strict"; WITH the quotes, and in particular with the letter u inside the quotes.

@@ +43,5 @@
> +function asyncCheckCacheEntryExistance(entryName, shouldExist)
> +{
> +  var listener = new CacheListener();
> +  listener.onCacheEntryAvailable = function(descriptor, accessGranted, status) {
> +    dump("foo");

Why do you have foo?
Comment 8 Matthias Versen [:Matti] 2012-06-19 12:36:04 PDT
*** Bug 766224 has been marked as a duplicate of this bug. ***
Comment 9 Marcia Knous [:marcia - use ni] 2012-06-25 06:37:40 PDT
On Firefox side:

mozilla::net::nsHttpChannel::InstallOfflineCacheListener() - 22 trunk/16 Aurora crashes in the last week.

mozilla::net::nsHttpChannel::InstallOfflineCacheListener  - 10 trunk

mozilla::net::nsHttpChannel::InstallOfflineCacheListener has more volume on FennecAndroid side so perhaps it doesn't have to be tracked on the desktop side for FF 15.
Comment 10 Jason Smith [:jsmith] 2012-06-26 16:16:05 PDT
k9o nomination - this causing crashes in web apps. App cache is likely to be used in web apps, so this is a likely use case.
Comment 11 Lawrence Mandel [:lmandel] (use needinfo) 2012-06-27 13:25:02 PDT
k9o- This issue is a quality issue that is already tracked for Fx15.
Comment 12 Justin Lebar (not reading bugmail) 2012-06-28 05:20:04 PDT
*** Bug 769211 has been marked as a duplicate of this bug. ***
Comment 13 JP Rosevear [:jpr] 2012-06-28 05:24:55 PDT
*** Bug 769210 has been marked as a duplicate of this bug. ***
Comment 14 JP Rosevear [:jpr] 2012-06-28 05:26:08 PDT
Any reason not to land the fix and get an uplift to Aurora?
Comment 15 Justin Lebar (not reading bugmail) 2012-06-28 05:28:11 PDT
If this is indeed the same as bug 769211, it's blocking an important b2g demo for tomorrow.  I'm going to verify that this fixes bug 769211; if so, we'll need this landed very soon (perhaps directly on m-c, if that's possible).
Comment 16 Justin Lebar (not reading bugmail) 2012-06-28 06:06:16 PDT
I verified that this fixes bug 769211.  I'd presume it also fixes bug 769210; I don't have a device or build handy to test on.

So there's less pressure to check this in, I've pushed the code fix to the B2G repository.  I didn't check in the tests because I didn't want to conflict with your review changes.  Anyway, we don't run xpcshell tests on b2g.

https://github.com/mozilla-b2g/mozilla-central/commit/4085977dd826686ca4ab6ffe023f61488773e1b9
Comment 17 Aaron Train [:aaronmt] 2012-06-28 06:59:12 PDT
This is breaking mobile Twitter if you opt-in to storing offline data.
Comment 18 JP Rosevear [:jpr] 2012-06-28 07:09:13 PDT
(In reply to Justin Lebar [:jlebar] from comment #16)
> So there's less pressure to check this in, I've pushed the code fix to the
> B2G repository.  

For B2G, but this is busting fennec native as well, right now.
Comment 19 Jason Smith [:jsmith] 2012-06-28 08:07:10 PDT
(In reply to JP Rosevear [:jpr] from comment #18)
> (In reply to Justin Lebar [:jlebar] from comment #16)
> > So there's less pressure to check this in, I've pushed the code fix to the
> > B2G repository.  
> 
> For B2G, but this is busting fennec native as well, right now.

And busting the destkop web runtime.
Comment 20 Justin Lebar (not reading bugmail) 2012-06-28 11:09:30 PDT
Andreas just asked me to land this patch on m-c.  I'm about to leave for dinner, but I'll do it when I get back if Brian doesn't get to it first.
Comment 21 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2012-06-28 15:31:32 PDT
https://hg.mozilla.org/mozilla-central/rev/90f4210aa8d6
Comment 22 Tony Chung [:tchung] 2012-06-29 01:21:41 PDT
Verified fix against 6-29-2012 b2g build, otoro device.  Twitter does not crash anymore. 

commit 4085977dd826686ca4ab6ffe023f61488773e1b9
Author: Brian Smith <bsmith@mozilla.com>
Date:   Thu Jun 28 08:45:07 2012 -0400

    Bug 760955: Fix null pointer deref in InstallOfflineCacheEntry, r=hurley
    
    Checked in directly to mozilla-b2g's git repository, due to bug 769211.  a=cjones over IRC.
Comment 23 Mark Finkle (:mfinkle) (use needinfo?) 2012-07-01 08:48:56 PDT
Brian or Justin - Can you request Aurora approval with the risk assessment? This crash is happening in Fennec 15 and we'd like to get it uplifted soon.
Comment 24 Justin Lebar (not reading bugmail) 2012-07-02 02:53:32 PDT
Brian made it clear to me that I have no idea what's going on here, so this is all on him.
Comment 25 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2012-07-03 15:40:48 PDT
I have asked for the change that caused the regression to be backed out of aurora in bug 722034.
Comment 26 Chris Peterson [:cpeterson] 2012-07-10 11:52:54 PDT
This crash is still the #1 topcrash on Fennec 15. The Aurora 15 backout landed just a few minutes ago, as per Bug 722034 Comment 79.
Comment 27 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2012-09-19 06:20:33 PDT
Comment on attachment 629677 [details] [diff] [review]
Fix null pointer deref in InstallOfflineCacheEntry

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 722034
User impact if declined: Crashes
Testing completed (on m-c, etc.): This patch has been on mozilla-central and aurora for a while now.
Risk to taking this patch (and alternatives if risky): This is a very, very safe patch.
String or UUID changes made by this patch: none.
Comment 28 Scoobidiver (away) 2012-09-19 09:17:39 PDT
The patch is already in 16 Beta and Aurora 17 (see the target milestone).
Comment 29 Lukas Blakk [:lsblakk] use ?needinfo 2012-09-19 10:18:35 PDT
Comment on attachment 629677 [details] [diff] [review]
Fix null pointer deref in InstallOfflineCacheEntry

Yup, this landed on central when 16 was still there, so no need to uplift.

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