Firefox Crash @ nsXMLHttpRequest::IsWaitingForHTMLCharset()

RESOLVED FIXED in mozilla11

Status

()

Core
DOM
--
critical
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: marcia, Assigned: hsivonen)

Tracking

({crash, regression})

Trunk
mozilla11
x86
Windows 7
crash, regression
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

6 years ago
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
Assignee: nobody → hsivonen
Blocks: 651072
Looks like doc is null here.
(Assignee)

Comment 2

6 years ago
Created attachment 576096 [details] [diff] [review]
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)?
Attachment #576096 - Flags: review?(bugs)
(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;};

Updated

6 years ago
Attachment #576096 - Flags: review?(bugs) → review+
(Assignee)

Comment 4

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

I'll prepare a crashtest from the suggestion in comment 3.
(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;};
(Assignee)

Comment 6

6 years ago
(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.
(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)
Created attachment 576146 [details] [diff] [review]
crashtest as a patch

Hm, my test case didn't work unless it was not loaded on HTTP.
Attachment #576146 - Flags: review?(hsivonen)
> unless it was not loaded on HTTP.
unless it was loaded on HTTP.
Created attachment 576149 [details] [diff] [review]
crashtest as a patch

Removed a bogus onload handler.
Attachment #576146 - Attachment is obsolete: true
Attachment #576146 - Flags: review?(hsivonen)
Attachment #576149 - Flags: review?(hsivonen)
Created attachment 576157 [details] [diff] [review]
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?
Attachment #576149 - Attachment is obsolete: true
Attachment #576149 - Flags: review?(hsivonen)
Attachment #576157 - Flags: review?(hsivonen)
https://hg.mozilla.org/mozilla-central/rev/3a00a7c1227f

Leaving open for crashtest.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla11
Generally we set in-testsuite to ? and close the bug in this case.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Duplicate of this bug: 704445

Updated

6 years ago
Crash Signature: [@ nsXMLHttpRequest::IsWaitingForHTMLCharset() ] → [@ nsXMLHttpRequest::IsWaitingForHTMLCharset() ] [@ nsXMLHttpRequest::IsWaitingForHTMLCharset ]

Comment 15

6 years ago
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()
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.
(Assignee)

Comment 17

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

Where's the JS identifier location defined in the crashtest?
(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.
(Assignee)

Comment 19

6 years ago
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.
Attachment #576157 - Flags: review?(hsivonen) → review+

Comment 20

6 years ago
(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.
Created attachment 576418 [details] [diff] [review]
crashtest as a patch for check in, r=hsivonen
Attachment #576157 - Attachment is obsolete: true
Attachment #576418 - Flags: review+
Keywords: checkin-needed
(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.
(Assignee)

Comment 23

6 years ago
Thanks. Test landed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1bca914c339e
Flags: in-testsuite? → in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/1bca914c339e
Keywords: checkin-needed
(Assignee)

Updated

6 years ago
Duplicate of this bug: 704760
You need to log in before you can comment on or make changes to this bug.