crash in mozilla::net::nsHttpChannel::InstallOfflineCacheListener

VERIFIED FIXED in Firefox 15

Status

()

Core
Networking: HTTP
--
critical
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: Scoobidiver (away), Assigned: briansmith)

Tracking

(4 keywords)

15 Branch
mozilla16
crash, regression, reproducible, topcrash
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-kilimanjaro:-, firefox12 unaffected, firefox13 unaffected, firefox14 unaffected, firefox15+ fixed)

Details

(Whiteboard: [native-crash],[fixed in b2g],[qa+], crash signature)

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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
Assignee: nobody → bsmith
Here's the offending changeset:
http://hg.mozilla.org/mozilla-central/rev/0f3f9ff4439d#l1.10
Blocks: 746018
OS: Android → All
Hardware: ARM → All
Version: 15 Branch → Trunk
Created attachment 629677 [details] [diff] [review]
Fix null pointer deref in InstallOfflineCacheEntry
Attachment #629677 - Flags: review?(hurley)
status-firefox12: --- → unaffected
status-firefox13: --- → unaffected
status-firefox14: --- → unaffected
status-firefox15: --- → affected
tracking-firefox15: --- → ?
Target Milestone: --- → Future
(Reporter)

Updated

5 years ago
Crash Signature: [@ mozilla::net::nsHttpChannel::InstallOfflineCacheListener] → [@ mozilla::net::nsHttpChannel::InstallOfflineCacheListener] [@ mozilla::net::nsHttpChannel::InstallOfflineCacheListener()]
(Reporter)

Comment 3

5 years ago
It's #1 top crasher in FennecAndroid 16.0a1.
tracking-fennec: --- → ?
Keywords: topcrash
STR: 
i. Install Aurora or Nightly on Android
ii. https://touch.betfair.com/, allow for offline data storage via prompt
(Reporter)

Updated

5 years ago
Keywords: reproducible
Duplicate of this bug: 763088
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
Attachment #629677 - Flags: review?(hurley) → review+
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?

Updated

5 years ago
tracking-firefox15: ? → +
Duplicate of this bug: 766224
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.
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.
blocking-kilimanjaro: --- → ?
k9o- This issue is a quality issue that is already tracked for Fx15.
blocking-kilimanjaro: ? → -
(Reporter)

Updated

5 years ago
Version: Trunk → 15 Branch
Duplicate of this bug: 769211

Updated

5 years ago
Duplicate of this bug: 769210

Comment 14

5 years ago
Any reason not to land the fix and get an uplift to Aurora?
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).
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
Whiteboard: [native-crash] → [native-crash][fixed in b2g]
This is breaking mobile Twitter if you opt-in to storing offline data.

Comment 18

5 years ago
(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.
Blocks: 763510
(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.
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.
https://hg.mozilla.org/mozilla-central/rev/90f4210aa8d6
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: Future → mozilla16
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.
Status: RESOLVED → VERIFIED

Updated

5 years ago
Whiteboard: [native-crash][fixed in b2g] → [native-crash],[fixed in b2g],[qa+]
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.
Brian made it clear to me that I have no idea what's going on here, so this is all on him.
I have asked for the change that caused the regression to be backed out of aurora in bug 722034.
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.
(Reporter)

Updated

5 years ago
Blocks: 722034
status-firefox15: affected → fixed
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.
Attachment #629677 - Flags: approval-mozilla-beta?
Attachment #629677 - Flags: approval-mozilla-aurora?
(Reporter)

Comment 28

5 years ago
The patch is already in 16 Beta and Aurora 17 (see the target milestone).
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.
Attachment #629677 - Flags: approval-mozilla-beta?
Attachment #629677 - Flags: approval-mozilla-beta-
Attachment #629677 - Flags: approval-mozilla-aurora?
Attachment #629677 - Flags: approval-mozilla-aurora-
Blocks: 767208
tracking-fennec: ? → ---
You need to log in before you can comment on or make changes to this bug.