1396300 - Crash in OOM | large | NS_ABORT_OOM | CopyUTF16toUTF8 | mozilla::dom::XMLHttpRequestMainThread::SendInternal

Status: RESOLVED FIXED

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

Description

[[:philipp]]

This bug was filed from the Socorro interface and is 
report bp-2a626d7f-d70c-4c28-9974-214810170902.
=============================================================
Crashing Thread (0)
Frame 	Module 	Signature 	Source
0 	xul.dll 	NS_ABORT_OOM(unsigned int) 	xpcom/base/nsDebugImpl.cpp:610
1 	xul.dll 	CopyUTF16toUTF8(nsAString const&, nsACString&) 	xpcom/string/nsReadableUtils.cpp:130
2 	xul.dll 	mozilla::dom::XMLHttpRequestMainThread::SendInternal(mozilla::dom::BodyExtractorBase const*) 	dom/xhr/XMLHttpRequestMainThread.cpp:2994
3 	xul.dll 	mozilla::dom::XMLHttpRequestMainThread::Send(JSContext*, nsAString const&, mozilla::ErrorResult&) 	dom/xhr/XMLHttpRequestMainThread.h:379
4 	xul.dll 	mozilla::dom::XMLHttpRequestBinding::send 	obj-firefox/dom/bindings/XMLHttpRequestBinding.cpp:783
5 	xul.dll 	mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) 	dom/bindings/BindingUtils.cpp:3053
6 	xul.dll 	js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp:469
7 	xul.dll 	js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) 	js/src/vm/Interpreter.cpp:533
8 	xul.dll 	js::fun_apply(JSContext*, unsigned int, JS::Value*) 	js/src/jsfun.cpp:1309
9 	xul.dll 	js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp:469
10 	xul.dll 	InternalCall 	js/src/vm/Interpreter.cpp:514
11 	xul.dll 	js::jit::DoCallFallback 	js/src/jit/BaselineIC.cpp:2589
12 		@0x245472a1 	
13 		@0x517841ef 	
14 		@0x2454094e 	
15 	xul.dll 	js::jit::EnterBaselineMethod(JSContext*, js::RunState&) 	js/src/jit/BaselineJIT.cpp:200
16 	xul.dll 	Interpret 	js/src/vm/Interpreter.cpp:3109
17 	xul.dll 	js::RunScript(JSContext*, js::RunState&) 	js/src/vm/Interpreter.cpp:409
18 	xul.dll 	js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp:487
19 	xul.dll 	js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) 	js/src/vm/Interpreter.cpp:533
20 	xul.dll 	js::fun_apply(JSContext*, unsigned int, JS::Value*) 	js/src/jsfun.cpp:1309
21 	xul.dll 	js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp:469
22 	xul.dll 	InternalCall 	js/src/vm/Interpreter.cpp:514
23 	xul.dll 	js::jit::DoCallFallback 	js/src/jit/BaselineIC.cpp:2589
24 		@0x245472a1 	
25 		@0xc4ac407 	
26 		@0x24564024 	
27 		@0x1a68a0b7 	
28 		@0x2454094e 	
29 	xul.dll 	js::jit::EnterBaselineMethod(JSContext*, js::RunState&) 	js/src/jit/BaselineJIT.cpp:200
30 	xul.dll 	js::RunScript(JSContext*, js::RunState&) 	js/src/vm/Interpreter.cpp:399
31 	xul.dll 	js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp:487
32 	xul.dll 	js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) 	js/src/vm/Interpreter.cpp:533
33 	xul.dll 	JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) 	js/src/jsapi.cpp:2949
34 	xul.dll 	mozilla::dom::AutoJSAPI::ReportException() 	dom/script/ScriptSettings.cpp:635

this crash signature is newly appearing since firefox 56 - there are no obvious correlations...

Comment 1

[Hsin-Yi Tsai (she/her) | away [:hsinyi]]

Hi Shawn, this seems a new regression in beta 56,  could you please take a look at this? Thanks.

Comment 2

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

https://hg.mozilla.org/releases/mozilla-beta/file/tip/dom/xhr/XMLHttpRequestMainThread.cpp#l2994

Comment 3

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

I try to reproduce it on Windows 7, but no luck. I'm still looking into how to reproduce this bug.

Comment 4

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

https://crash-stats.mozilla.com/report/index/f5e859ac-5930-4a3f-bba2-e99610170906
OOM Allocation Size is quite large 13MB. System Memory Use Percentage is 95%.

Comment 5

[Nathan Froyd [:froydnj]]

I think this call is failing:

http://dxr.mozilla.org/mozilla-central/source/dom/fetch/BodyExtractor.cpp#138

That call should be made fallible.

Comment 6

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: Bug 1396300 - Use fallible allocation in GetAsStream

Comment 7

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

(In reply to Nathan Froyd [:froydnj] from comment #5)
> I think this call is failing:
> 
> http://dxr.mozilla.org/mozilla-central/source/dom/fetch/BodyExtractor.cpp#138
> 
> That call should be made fallible.

Thank you. Yeah, this was introduced in bug 1261841, that explained why we saw it from 56 beta.

Comment 8

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8905403 [details] [diff] [review]
Bug 1396300 - Use fallible allocation in GetAsStream

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

::: dom/fetch/BodyExtractor.cpp
@@ +134,5 @@
>                                              nsACString& aContentTypeWithCharset,
>                                              nsACString& aCharset) const
>  {
>    nsCString encoded;
> +  if (!CopyUTF16toUTF8(*mBody, encoded, fallible)) {

I wish we had a way to do the UTF16-to-UTF8 conversion as part of the stream reading process.  I guess that can be a follow-up bug, though.

Comment 9

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Opened Bug 1398003 for follow-up.

Comment 10

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: Bug 1396300 - Use fallible allocation in GetAsStream. r=bkelly

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6b1112ce0727
Use fallible allocation in GetAsStream. r=bkelly

Comment 12

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Comment on attachment 8905765 [details] [diff] [review]
Bug 1396300 - Use fallible allocation in GetAsStream. r=bkelly

Approval Request Comment
[Feature/Bug causing the regression]:bug 1261841
[User impact if declined]:Crash in OOM
[Is this code covered by automated tests?]:No
[Has the fix been verified in Nightly?]:No STR
[Needs manual test from QE? If yes, steps to reproduce]: No STR
[List of other uplifts needed for the feature/fix]:No
[Is the change risky?]:Very Low
[Why is the change risky/not risky?]:Use fallible allocation
[String changes made/needed]:No

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/6b1112ce0727

Comment 14

[Gerry Chang [:gchang]]

Comment on attachment 8905765 [details] [diff] [review]
Bug 1396300 - Use fallible allocation in GetAsStream. r=bkelly

Fix a crash. Let's see how it goes. Beta56+.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d65474a88a77

Comment 16

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Thanks, Ryan.