813947 - crash in nsDocumentOpenInfo::OnStartRequest

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect, P1)

Description

[Scoobidiver (away)]

It's currently #17 top browser crasher in 17.0.

Signature 	nsDocumentOpenInfo::OnStartRequest(nsIRequest*, nsISupports*) More Reports Search
UUID	8400a6a4-d3c1-4f13-b454-3f24d2121121
Date Processed	2012-11-21 10:27:47
Uptime	5320
Last Crash	7.0 weeks before submission
Install Age	15.5 hours since version was first installed.
Install Time	2012-11-20 18:56:06
Product	Firefox
Version	17.0
Build ID	20121119183901
Release Channel	release
OS	Windows NT
OS Version	5.1.2600 Service Pack 2
Build Architecture	x86
Build Architecture Info	GenuineIntel family 15 model 4 stepping 7
Crash Reason	EXCEPTION_ACCESS_VIOLATION_READ
Crash Address	0x0
App Notes 	
AdapterVendorID: 0x1002, AdapterDeviceID: 0x954f, AdapterSubsysID: 21431002, AdapterDriverVersion: 8.640.0.0
D3D10 Layers? D3D10 Layers- D3D9 Layers? D3D9 Layers- 
EMCheckCompatibility	True
Adapter Vendor ID	0x1002
Adapter Device ID	0x954f
Total Virtual Memory	2147352576
Available Virtual Memory	1741447168
System Memory Use Percentage	82
Available Page File	1701875712
Available Physical Memory	188301312

Frame 	Module 	Signature 	Source
0 	xul.dll 	nsDocumentOpenInfo::OnStartRequest 	uriloader/base/nsURILoader.cpp:248
1 	xul.dll 	nsObjectLoadingContent::LoadObject 	content/base/src/nsObjectLoadingContent.cpp:1868
2 	xul.dll 	nsObjectLoadingContent::OnStartRequest 	content/base/src/nsObjectLoadingContent.cpp:847
3 	xul.dll 	mozilla::net::nsHttpChannel::CallOnStartRequest 	netwerk/protocol/http/nsHttpChannel.cpp:964
4 	xul.dll 	mozilla::net::nsHttpChannel::ContinueOnStartRequest2 	netwerk/protocol/http/nsHttpChannel.cpp:4826
5 	xul.dll 	nsAppShell::ProcessNextNativeEvent 	widget/windows/nsAppShell.cpp:339
6 	xul.dll 	nsInputStreamPump::OnInputStreamReady 	netwerk/base/src/nsInputStreamPump.cpp:368
7 	xul.dll 	nsInputStreamReadyEvent::Run 	xpcom/io/nsStreamUtils.cpp:82
8 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:624
9 	xul.dll 	mozilla::ipc::MessagePump::Run 	ipc/glue/MessagePump.cpp:82
10 	xul.dll 	MessageLoop::RunHandler 	ipc/chromium/src/base/message_loop.cc:201
11 	xul.dll 	MessageLoop::Run 	ipc/chromium/src/base/message_loop.cc:175
12 	xul.dll 	nsBaseAppShell::Run 	widget/xpwidgets/nsBaseAppShell.cpp:163
13 	xul.dll 	nsAppShell::Run 	widget/windows/nsAppShell.cpp:232
14 	xul.dll 	nsAppStartup::Run 	toolkit/components/startup/nsAppStartup.cpp:273
15 	xul.dll 	XREMain::XRE_mainRun 	toolkit/xre/nsAppRunner.cpp:3812
16 	xul.dll 	XREMain::XRE_main 	toolkit/xre/nsAppRunner.cpp:3889
17 	xul.dll 	XRE_main 	toolkit/xre/nsAppRunner.cpp:3965
18 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp:100
19 	firefox.exe 	__tmainCRTStartup 	crtexe.c:552
20 	kernel32.dll 	BaseProcessStart 	

More reports at:
https://crash-stats.mozilla.com/report/list?signature=nsDocumentOpenInfo%3A%3AOnStartRequest%28nsIRequest*%2C+nsISupports*%29

Comment 1

[Benjamin Smedberg]

Pretty certain http://hg.mozilla.org/releases/mozilla-release/annotate/919435c6f654/content/base/src/nsObjectLoadingContent.cpp#l1868 is calling OnStartRequest with a null mChannel. Marcia, this should have URLs, can you see if there's anything useful?

Comment 2

[Marcia Knous [:marcia]]

Correlations from 17 show this is related to Norton:

nsDocumentOpenInfo::OnStartRequest(nsIRequest*, nsISupports*)|EXCEPTION_ACCESS_VIOLATION_READ (78 crashes)
    100% (78/78) vs.   1% (492/43394) {2D3F3651-74B9-4795-BDEC-6DA2F431CB62}
     74% (58/78) vs.   2% (715/43394) {BBDA0591-3099-440a-AA10-41764D9DB4DB}
     22% (17/78) vs.   3% (1218/43394) {CAFEEFAC-0016-0000-0035-ABCDEFFEDCBA}
     19% (15/78) vs.   2% (1041/43394) {CAFEEFAC-0016-0000-0033-ABCDEFFEDCBA}
     19% (15/78) vs.   3% (1218/43394) {CAFEEFAC-0016-0000-0037-ABCDEFFEDCBA}
     22% (17/78) vs.   6% (2484/43394) {635abd67-4fe9-1b23-4f01-e679fa7484c1} (Yahoo! Toolbar, https://addons.mozilla.org/addon/2032)
     13% (10/78) vs.   4% (1800/43394) {b9db16a4-6edc-47ec-a1f4-b86292ed211d} (Video DownloadHelper, https://addons.mozilla.org/addon/3006)
      8% (6/78) vs.   0% (146/43394) [e-mail]
      8% (6/78) vs.   0% (173/43394) [e-mail]
     99% (77/78) vs.  93% (40195/43394) {972ce4c6-7e08-4474-a285-3208198ce6fd} (Default, https://addons.mozilla.org/addon/8150)

Comment 3

[John Schoenick [:johns]]

LoadObject checks that it was called from OnStartRequest for channel loads, and sanity-checks that it has a channel at that. if mChannel is null at this point, we probably re-entered somewhere. Possibly something Norton hooks that wouldn't otherwise be re-entrant.

Comment 4

[Georg Fritzsche [:gfritzsche]]

(In reply to Marcia Knous [:marcia] from comment #2)
> Correlations from 17 show this is related to Norton:

Can you tell if it's any specific Norton product? I assume there are no specific URLs standing out?

Comment 5

[Georg Fritzsche [:gfritzsche]]

(In reply to Georg Fritzsche [:gfritzsche] from comment #4)
> I assume there are no specific URLs standing out?

Nevermind, the comments point to some variants worth testing on Amazon.

Comment 6

[Scoobidiver (away)]

(In reply to Georg Fritzsche [:gfritzsche] from comment #4)
> Can you tell if it's any specific Norton product?
It's Norton Confidential 2012:
    100% (39/39) vs.   2% (134/8801) couictlr.dll
        100% (39/39) vs.   1% (103/8801) 2012.5.6.10
          0% (0/39) vs.   0% (31/8801) 2013.2.0.18

Comment 7

[Georg Fritzsche [:gfritzsche]]

From what i gathered, Confidential seems to be part of Nortons "Internet Security". There is a trial, but is a bit too agressive for me here as it interferes with other development.

Comment 8

[Georg Fritzsche [:gfritzsche]]

Attachment: Null check

Going for a null check until we have more information.

John, do you see any problems with this? It should be handled properly from nsObjectLoadingContents side as far as i can tell.

Comment 9

[John Schoenick [:johns]]

Comment on attachment 685228 [details] [diff] [review]
Null check

I have a feeling something may be broken in nsObjectLoadingContent since bug 745030 and a ton of follow-ups landed on 17.

It may be that Norton is hooking OpenChannel and causing re-entrance:
> http://dxr.mozilla.org/mozilla-central/content/base/src/nsObjectLoadingContent.cpp.html#l1798

We don't check that call for re-entrance, and we may call OnStartRequest if finalListener is set, even if NS_FAILED(rv)

A null check here is a good idea either way though, maybe even make it assert as well.

Comment 10

[Georg Fritzsche [:gfritzsche]]

Attachment: Null check, v2

Good point about the assertion.
https://tbpl.mozilla.org/?tree=Try&rev=dc76702e6d40

Benjamin, as a docshell peer, can you review this?

Comment 11

[Lukas Blakk [:lsblakk] use ?needinfo]

Tracking since we could consider this for a 17.0.1 ridealong.

Comment 12

[Marcia Knous [:marcia]]

I will try to reproduce this on Windows, but this product is part of the Norton Internet security suite. I tried reproducing the Mac specific version of it and had no luck. Just like the Mac crashes, most of the reports have a fair amount of uptime.

Comment 13

[Marcia Knous [:marcia]]

Unfortunately, I keep getting a message "The page isn't redirecting properly" when I try to download Norton from Firefox or any other browser in my VM - clearing cookies etc doesn't help. So I am blocked on testing this further until I can download it.

Comment 14

[Georg Fritzsche [:gfritzsche]]

(In reply to Marcia Knous [:marcia] from comment #13)
> Unfortunately, I keep getting a message "The page isn't redirecting
> properly"

Weird, still worked for me on monday - i only have the german setup stored here though. I have contacted their support.

Comment 15

[Robert Kaiser]

Here's the top 10 URLs for this - there would be more, but almost everything is Amazon anyhow.

54 	http://www.amazon.com/gp/product/handle-buy-box/ref=dp_start-bbf_1_glance
33 	http://www.amazon.com/gp/cart/view.html/ref=gno_cart
19 	http://www.amazon.com/gp/prime/handle-buy-box.html/ref=dp_start-bbf_1_glance
19 	http://www.amazon.com/
19 	https://www.amazon.com/gp/digital/fiona/buy.html
18 	https://www.amazon.com/gp/css/homepage.html/ref=gno_yam_ya
15 	http://www.amazon.de/gp/product/handle-buy-box/ref=dp_start-bbf_1_glance
14 	https://images-na.ssl-images-amazon.com/images/G/01/browser-scripts/site-wide-js
13 	https://www.amazon.com/gp/css/order-history/ref=gno_yam_yrdrs
11 	http://www.amazon.com/gp/goldbox/ref=cs_top_nav_gb27

Comment 16

[John Schoenick [:johns]]

Attachment: Don't try to call OnStartRequest on failures in nsObjectLoadingContent::LoadObject

So if [1] fills in finalListener but returns a failure code, we will close the
channel and switch to type null, but leave the finalListener around, which we
will inappropriately try to call OnStartRequest on.

As a bonus ride-along, we should clean up the dangling channel if
mFinalListener->OnStartRequest fails

Combined with Georg's patch this should probably make things better, but we
still don't have anyone who can reproduce :(

Comment 17

[Benjamin Smedberg]

Comment on attachment 685305 [details] [diff] [review]
Null check, v2

Please don't use NS_ENSURE_*; it hides a return and a warning. Instead write it out:

MOZ_ASSERT(request);
if (request) {
  return NS_ERROR_UNEXPECTED;
}

r=me with that change

Comment 18

[Marcia Knous [:marcia]]

Norton-US is back up, so downloading this now to try to repro.

Comment 19

[John Schoenick [:johns]]

Try build:
  https://tbpl.mozilla.org/?tree=Try&rev=8e3cecd98155

Comment 20

[u279076]

I tried testing this with Firefox 17.0 and Norton Internet Security 2013 on a Windows 7 32-bit native machine but was unable to get this to reproduce. Hopefully Marcia can come up with something.

Comment 21

[Lukas Blakk [:lsblakk] use ?needinfo]

Thanks for trying - we're going to move forward with the 17.0.1 respin without this fix.  If it becomes a significant crash issue in branches, please nominate for tracking as appropriate.

Comment 22

[Marcia Knous [:marcia]]

nsDocumentOpenInfo::OnStartRequest(nsIRequest*, nsISupports*)|EXCEPTION_ACCESS_VIOLATION_READ (20 crashes)
    100% (20/20) vs.   1% (135/9962) {2D3F3651-74B9-4795-BDEC-6DA2F431CB62}
         15% (3/20) vs.   0% (20/9962) 2012.5.8.3
         85% (17/20) vs.   1% (78/9962) 2012.5.8.4
          0% (0/20) vs.   0% (1/9962) 2013.2.0.18
          0% (0/20) vs.   0% (6/9962) 2013.2.2.2
          0% (0/20) vs.   0% (30/9962) 2013.2.2.3
     70% (14/20) vs.   2% (164/9962) {BBDA0591-3099-440a-AA10-41764D9DB4DB} (11.1.1.5 - 3)

I just downloaded Norton Internet Suite but I wasn't able to get the crashy version installed - 2012.5.8.4. Looking now to see more about this version.

Comment 23

[Georg Fritzsche [:gfritzsche]]

Attachment: Null check, v3

https://hg.mozilla.org/integration/mozilla-inbound/rev/938cc49bbb1a

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/938cc49bbb1a

Comment 25

[Georg Fritzsche [:gfritzsche]]

Not fixed yet, that patch just adresses a symptom.

Comment 26

[John Schoenick [:johns]]

Comment on attachment 686259 [details] [diff] [review]
Don't try to call OnStartRequest on failures in nsObjectLoadingContent::LoadObject

https://hg.mozilla.org/integration/mozilla-inbound/rev/2880c2eca430

Comment 27

[Alex Keybl [:akeybl]]

We fully expect this to be an issue again at the end of 18.0beta. Do we have a plan for verifying, given the fact that this likely won't be a top crasher until the end of 18.0beta?

Comment 28

[John Schoenick [:johns]]

Unless QA can figure out STR, this is a speculative fix.

I'm moderately confident that my patch solves this problem, and 99% sure Georg's null check will at least avoid a crash (and probably fully fix the problem depending on what exactly norton is doing). Both are fairly low risk.

Comment 29

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 686647 [details] [diff] [review]
Null check, v3

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Unknown, possibly refactorings & Norton combined.
User impact if declined: continued browser crashes for a user subset.
Testing completed (on m-c, etc.): landed fine on m-c.
Risk to taking this patch (and alternatives if risky): low risk, just a null check.
String or UUID changes made by this patch: none.

Comment 30

[John Schoenick [:johns]]

Comment on attachment 686259 [details] [diff] [review]
Don't try to call OnStartRequest on failures in nsObjectLoadingContent::LoadObject

[Approval Request Comment]
Bug caused by (feature/regressing bug #):
Norton bad touch probably, aggravated by bug 745030 refactoring or its follow-ups.

Testing completed (on m-c, etc.): 
on m-c

If this is not a sec:{high,crit} bug, please state case for ESR consideration:
Low risk (speculative) fix for a 17 topcrash

User impact if declined: 
topcrash

Fix Landed on Version:
20, likely uplift for 18+

Risk to taking this patch (and alternatives if risky): 
Low, fixes simple check in plugin code. Alternative is just taking Georg's null check, which will probably also stop the crash on its own, though we have no STR.

String or UUID changes made by this patch: 
None

Comment 31

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 686647 [details] [diff] [review]
Null check, v3

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
Low risk null-check that should guard against a top-crasher.
User impact if declined: see comment 29
Fix Landed on Version: 20, uplift to 18/19 pending
Risk to taking this patch (and alternatives if risky): low risk, just a null check
String or UUID changes made by this patch: none.

Comment 32

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2880c2eca430

Comment 33

[Lukas Blakk [:lsblakk] use ?needinfo]

This is not an ESR topcrasher (granted, we likely get very little crash data from ESR deployments) but fixing something like this in ESR is not part of the criteria so we wouldn't track/fix this without a sign that there was a lot of user demand.

Comment 34

[John Schoenick [:johns]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/630047f6f009
https://hg.mozilla.org/releases/mozilla-aurora/rev/06236ab191ec

https://hg.mozilla.org/releases/mozilla-beta/rev/e9d2055f9ea8
https://hg.mozilla.org/releases/mozilla-beta/rev/c8f47b36129d

We have no good way to prove this bug fixed other than wait for another release, and are fairly confident that these patches will solve the problem. I'm going to mark this fixed until proven otherwise.

Comment 35

[Virgil Dicu [:virgil] [QA]]

Looks good, no crashes for Firefox 18 beta.
https://crash-stats.mozilla.com/report/list?signature=nsDocumentOpenInfo%3A%3AOnStartRequest%28nsIRequest*%2C+nsISupports*%29

Comment 36

[Virgil Dicu [:virgil] [QA]]

Actually reverting until I can check for a later beta. Misread the date.

Comment 37

[Virgil Dicu [:virgil] [QA]]

No crashes in beta 18 since the patch landed. Setting to verified.

No crashes at all for F19 and F20 for the last two weeks.