704284 - Firefox Crash @ nsXMLHttpRequest::IsWaitingForHTMLCharset()

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Marcia Knous [:marcia]]

Seen while looking at Windows trunk data. Crashes started showing up in crash stats using the 2011112100 build. https://crash-stats.mozilla.com/report/list?signature=nsXMLHttpRequest::IsWaitingForHTMLCharset%28%29 to the crashes - many seem to be one user but there is another OS with no service pack.

Possible pushlog regression range based on crash stats:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7dd46087e678&tochange=78cd6a30e250

https://crash-stats.mozilla.com/report/index/2ae44bbb-2edd-4d36-82b5-cf1622111121

Frame 	Module 	Signature [Expand] 	Source
0 	xul.dll 	nsXMLHttpRequest::IsWaitingForHTMLCharset 	content/base/src/nsXMLHttpRequest.cpp:1483
1 	xul.dll 	xpc_qsUnwrapThis<nsIXMLHttpRequest> 	js/xpconnect/src/XPCQuickStubs.h:536
2 	xul.dll 	nsXMLHttpRequest::GetResponseText 	content/base/src/nsXMLHttpRequest.cpp:888
3 	xul.dll 	nsIXMLHttpRequest_GetResponseText 	obj-firefox/js/xpconnect/src/dom_quickstubs.cpp:22477
4 	xul.dll 	js::ContextStack::currentScript 	js/src/vm/Stack-inl.h:645
5 	xul.dll 	js::Shape::get 	js/src/jsscopeinlines.h:295
6 	xul.dll 	js_GetPropertyHelper 	js/src/jsobj.cpp:5944
7 	xul.dll 	js::Interpret 	js/src/jsinterp.cpp:3487
8 	xul.dll 	JSCompartment::wrap 	js/src/jscompartment.cpp:214
9 	xul.dll 	XPCNativeInterface::GetNewOrUsed 	js/xpconnect/src/XPCWrappedNativeInfo.cpp:162
10 	xul.dll 	XPCCallContext::Init 	js/xpconnect/src/XPCCallContext.cpp:148
11 	xul.dll 	XPCConvert::NativeInterface2JSObject 	js/xpconnect/src/XPCConvert.cpp:1302
12 	xul.dll 	XPCCallContext::XPCCallContext 	js/xpconnect/src/XPCCallContext.cpp:63
13 	xul.dll 	NS_IsMainThread_P 	obj-firefox/xpcom/build/nsThreadUtils.cpp:138
14 	xul.dll 	nsXPConnect::GetNativeOfWrapper 	js/xpconnect/src/nsXPConnect.cpp:1507
15 	xul.dll 	nsCycleCollector::Forget2 	xpcom/base/nsCycleCollector.cpp:2532
16 	xul.dll 	NS_IsMainThread_P 	obj-firefox/xpcom/build/nsThreadUtils.cpp:138
17 	xul.dll 	nsCOMPtr_base::assign_from_qi 	obj-firefox/xpcom/build/nsCOMPtr.cpp:96
18 	xul.dll 	XPCWrappedNative::InitTearOff 	js/xpconnect/src/XPCWrappedNative.cpp:2023
19 	xul.dll 	JS_NewObject 	js/src/jsapi.cpp:3236
20 	nspr4.dll 	MD_CURRENT_THREAD 	nsprpub/pr/src/md/windows/w95thred.c:308
21 	nspr4.dll 	MD_CURRENT_THREAD 	nsprpub/pr/src/md/windows/w95thred.c:308
22 	nspr4.dll 	PR_Unlock 	nsprpub/pr/src/threads/combined/prulock.c:347
23 	xul.dll 	FinishCreate 	js/xpconnect/src/XPCWrappedNative.cpp:633
24 	xul.dll 	nsCOMPtr_base::~nsCOMPtr_base 	obj-firefox/xpcom/build/nsCOMPtr.cpp:81
25 	xul.dll 	XPCWrappedNative::GetNewOrUsed 	js/xpconnect/src/XPCWrappedNative.cpp:584
26 	xul.dll 	JS_ObjectToInnerObject 	js/src/jsobj.cpp:142
27 	xul.dll 	JSCompartment::wrap 	js/src/jscompartment.cpp:214
28 	nspr4.dll 	PR_Unlock 	nsprpub/pr/src/threads/combined/prulock.c:347
29 	xul.dll 	JSCompartment::wrap 	js/src/jscompartment.cpp:359
30 	xul.dll 	XPCNativeInterface::GetNewOrUsed 	js/xpconnect/src/XPCWrappedNativeInfo.cpp:162
31 	xul.dll 	JS_WrapObject 	js/src/jsapi.cpp:1443
32 	xul.dll 	XPCConvert::NativeInterface2JSObject 	js/xpconnect/src/XPCConvert.cpp:1302
33 	xul.dll 	nsCOMPtr_base::~nsCOMPtr_base 	obj-firefox/xpcom/build/nsCOMPtr.cpp:81
34 	xul.dll 	XPCConvert::NativeData2JS 	js/xpconnect/src/XPCConvert.cpp:500
35 	xul.dll 	js::types::TypeScript::SetArgument 	js/src/jsinferinlines.h:682
36 	xul.dll 	js::types::TypeScript::SetThis 	js/src/jsinferinlines.h:631
37 	xul.dll 	js::ContextStack::pushInvokeFrame 	js/src/vm/Stack.cpp:691
38 	xul.dll 	js::InvokeKernel 	js/src/jsinterp.cpp:647
39 	xul.dll 	JSCompartment::wrap 	js/src/jscompartment.cpp:220
40 	xul.dll 	js::Invoke 	js/src/jsinterp.cpp:679
41 	nspr4.dll 	PR_Lock 	nsprpub/pr/src/threads/combined/prulock.c:223
42 	xul.dll 	JS_CallFunctionValue 	js/src/jsapi.cpp:5223
43 	xul.dll 	nsXPCWrappedJSClass::CallMethod 	js/xpconnect/src/XPCWrappedJSClass.cpp:1530
44 	xul.dll 	nsUrlClassifierHash<4>::FromPlaintext 	toolkit/components/url-classifier/nsUrlClassifierDBService.cpp:256

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Looks like doc is null here.

Comment 2

[Henri Sivonen (:hsivonen)]

Attachment: Null-check mResponseXML

I failed to realize that in some error cases, mResponseXML gets nulled out even in the HTML mode.

Does anyone have a minimized test for provoking this crash (for landing as a crashtest)?

Comment 3

[Masatoshi Kimura [:emk]]

(In reply to Henri Sivonen (:hsivonen) from comment #2)
> Does anyone have a minimized test for provoking this crash (for landing as a
> crashtest)?
var xhr=new XMLHttpRequest();xhr.open("GET",location);xhr.send();xhr.onprogress=function(){xhr.abort();};xhr.onabort=function(){xhr.responseText;};

Comment 4

[Henri Sivonen (:hsivonen)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3a00a7c1227f

I'll prepare a crashtest from the suggestion in comment 3.

Comment 5

[Masatoshi Kimura [:emk]]

(In reply to Henri Sivonen (:hsivonen) from comment #2)
> XMLHttpRequest();xhr.open("GET",location);xhr.send();xhr.
> onprogress=function(){xhr.abort();};xhr.onabort=function(){xhr.responseText;
> };
Sorry, the above test will not work reliably if the server did not send the response body in 50ms. Try this:
var xhr=new XMLHttpRequest();xhr.open("GET",location);xhr.send();xhr.onprogress=function(){if(xhr.readyState==3)xhr.abort();};xhr.onabort=function(){xhr.responseText;};

Comment 6

[Henri Sivonen (:hsivonen)]

(In reply to Masatoshi Kimura [:emk] from comment #5)
> Sorry, the above test will not work reliably if the server did not send the
> response body in 50ms. Try this:

I'm unable to make a crashtest that actually crashes with the fix using either of the suggestions.

Comment 7

[Masatoshi Kimura [:emk]]

(In reply to Henri Sivonen (:hsivonen) from comment #6)
> (In reply to Masatoshi Kimura [:emk] from comment #5)
> > Sorry, the above test will not work reliably if the server did not send the
> > response body in 50ms. Try this:
> 
> I'm unable to make a crashtest that actually crashes with the fix using
> either of the suggestions.
I could reproduce a crash by typing my suggestion into Web Console without the fix.
(Didn't test with fix, but it's natural it is not reproducable on fixed build)

Comment 8

[Masatoshi Kimura [:emk]]

Attachment: crashtest as a patch

Hm, my test case didn't work unless it was not loaded on HTTP.

Comment 9

[Masatoshi Kimura [:emk]]

> unless it was not loaded on HTTP.
unless it was loaded on HTTP.

Comment 10

[Masatoshi Kimura [:emk]]

Attachment: crashtest as a patch

Removed a bogus onload handler.

Comment 11

[Masatoshi Kimura [:emk]]

Attachment: crashtest as a patch, v3

I had to force MIME type to "text/html" by using overrideMimeType to reproduce the crash on local file. Is this intentional?

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/3a00a7c1227f

Leaving open for crashtest.

Comment 13

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Generally we set in-testsuite to ? and close the bug in this case.

Comment 15

[shailajajuluru]

Firefox 11.0a1(update 21st Nov, 2011 and 22nd Nov, 2011)crashes several times.

Crash Occurred during the following events.

1.Closing the Firefox by clicking close button on the right side.
2.Logging out Gmail account. 

Crash IDS :
bp-b35185b8-302a-44c4-804b-c855a2111122
bp-2ca9131e-4601-4ffb-9590-138872111122

Crash Signature :
nsXMLHttpRequest::IsWaitingForHTMLCharset()

Comment 16

[Tony Mechelynck [:tonymec]]

Mozilla/5.0 (X11; Linux x86_64; rv:11.0a1) Gecko/20111122 Firefox/11.0a1 SeaMonkey/2.8a1 ID:20111122003001

I suspect that crash bp-0f400737-c0fb-462b-ab00-bceb92111122 (in a Linux64 SeaMonkey nightly built some 9 hours before comment #12) might be due to the same problem. At least it has the same crash signature. Let's install the latest tinderbox-build, just in case. Of course, for a sporadic crash, only negative verification is possible.

Comment 17

[Henri Sivonen (:hsivonen)]

Comment on attachment 576157 [details] [diff] [review]
crashtest as a patch, v3

Where's the JS identifier location defined in the crashtest?

Comment 18

[Masatoshi Kimura [:emk]]

(In reply to Henri Sivonen (:hsivonen) from comment #17)
> Comment on attachment 576157 [details] [diff] [review] [diff] [details] [review]
> crashtest as a patch, v3
> 
> Where's the JS identifier location defined in the crashtest?
It's a location property of the global object. That is, it's equivalent to window.location.

Comment 19

[Henri Sivonen (:hsivonen)]

Comment on attachment 576157 [details] [diff] [review]
crashtest as a patch, v3

(In reply to Masatoshi Kimura [:emk] from comment #18)
> (In reply to Henri Sivonen (:hsivonen) from comment #17)
> > Comment on attachment 576157 [details] [diff] [review] [diff] [details] [review] [diff] [details] [review]
> > crashtest as a patch, v3
> > 
> > Where's the JS identifier location defined in the crashtest?
> It's a location property of the global object. That is, it's equivalent to
> window.location.

Oh, right. That's not particularly obvious. Would be clearer to use window.location.

Comment 20

[Scoobidiver (away)]

(In reply to shailajajuluru from comment #15)
> Firefox 11.0a1(update 21st Nov, 2011 and 22nd Nov, 2011)crashes several
> times.
It's normal because it's fixed in 11.0a1/20111123 which is not yet released.

Comment 21

[Masatoshi Kimura [:emk]]

Attachment: crashtest as a patch for check in, r=hsivonen

Comment 22

[Masatoshi Kimura [:emk]]

(In reply to Henri Sivonen (:hsivonen) from comment #19)
> > > Where's the JS identifier location defined in the crashtest?
> > It's a location property of the global object. That is, it's equivalent to
> > window.location.
> 
> Oh, right. That's not particularly obvious. Would be clearer to use
> window.location.
Fixed. Could you push the patch? I have no commit priv to the main tree.

Comment 23

[Henri Sivonen (:hsivonen)]

Thanks. Test landed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1bca914c339e

Comment 24

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1bca914c339e