1331606 - Crash in OOM | unknown | js::AutoEnterOOMUnsafeRegion::crash | JSRuntime::createJitRuntime

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[[:philipp]]

[Tracking Requested - why for this release]:
another late breaking crash regression in the 51 beta channel. it's accounting for 1.2% of all crashes in 51.0b14 at the moment.

This bug was filed from the Socorro interface and is 
report bp-2f7738ea-90ac-4012-a31f-345db2170117.
=============================================================
Crashing Thread (22)
Frame 	Module 	Signature 	Source
0 	xul.dll 	js::AutoEnterOOMUnsafeRegion::crash(char const*) 	js/src/jscntxt.cpp:1188
1 	xul.dll 	JSRuntime::createJitRuntime(JSContext*) 	js/src/jscompartment.cpp:182
2 	xul.dll 	JS::Zone::createJitZone(JSContext*) 	js/src/gc/Zone.cpp:283
3 	xul.dll 	JSCompartment::ensureJitCompartmentExists(JSContext*) 	js/src/jscompartment.cpp:195
4 	xul.dll 	js::jit::CanEnterBaselineMethod(JSContext*, js::RunState&) 	js/src/jit/BaselineJIT.cpp:397
5 	xul.dll 	js::RunScript(JSContext*, js::RunState&) 	js/src/vm/Interpreter.cpp:390
6 	xul.dll 	js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp:476
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:522
8 	xul.dll 	IntlInitialize 	js/src/builtin/Intl.cpp:546
9 	xul.dll 	CreateCollatorPrototype 	js/src/builtin/Intl.cpp:857
10 	xul.dll 	js::GlobalObject::initIntlObject(JSContext*, JS::Handle<js::GlobalObject*>) 	js/src/builtin/Intl.cpp:2505
11 	xul.dll 	js::InitIntlClass(JSContext*, JS::Handle<JSObject*>) 	js/src/builtin/Intl.cpp:2549
12 	xul.dll 	js::GlobalObject::resolveConstructor(JSContext*, JS::Handle<js::GlobalObject*>, JSProtoKey) 	js/src/vm/GlobalObject.cpp:170
13 	xul.dll 	js::GlobalObject::initStandardClasses(JSContext*, JS::Handle<js::GlobalObject*>) 	js/src/vm/GlobalObject.cpp:429
14 	xul.dll 	JS_InitStandardClasses(JSContext*, JS::Handle<JSObject*>) 	js/src/jsapi.cpp:941
15 	xul.dll 	mozilla::dom::CreateGlobal<mozilla::dom::DedicatedWorkerGlobalScope, &mozilla::dom::DedicatedWorkerGlobalScopeBinding::GetProtoObjectHandle(JSContext*)>(JSContext*, mozilla::dom::DedicatedWorkerGlobalScope*, nsWrapperCache*, JSClass const*, JS::CompartmentOptions&, JSPrincipals*, bool, JS::MutableHandle<JSObject*>) 	obj-firefox/dist/include/mozilla/dom/BindingUtils.h:2954
16 	xul.dll 	mozilla::dom::DedicatedWorkerGlobalScopeBinding::Wrap(JSContext*, mozilla::dom::DedicatedWorkerGlobalScope*, nsWrapperCache*, JS::CompartmentOptions&, JSPrincipals*, bool, JS::MutableHandle<JSObject*>) 	obj-firefox/dom/bindings/DedicatedWorkerGlobalScopeBinding.cpp:425
17 	xul.dll 	mozilla::dom::DedicatedWorkerGlobalScope::WrapGlobalObject(JSContext*, JS::MutableHandle<JSObject*>) 	dom/workers/WorkerScope.cpp:513
18 	xul.dll 	mozilla::dom::workers::WorkerPrivate::GetOrCreateGlobalScope(JSContext*) 	dom/workers/WorkerPrivate.cpp:6481
19 	xul.dll 	`anonymous namespace'::ScriptExecutorRunnable::PreRun 	dom/workers/ScriptLoader.cpp:1878
20 	xul.dll 	mozilla::dom::workers::WorkerRunnable::Run() 	dom/workers/WorkerRunnable.cpp:268
21 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp:1067
22 	xul.dll 	NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/glue/nsThreadUtils.cpp:311
23 	xul.dll 	mozilla::dom::workers::WorkerPrivate::RunCurrentSyncLoop() 	dom/workers/WorkerPrivate.cpp:5403
24 	xul.dll 	`anonymous namespace'::LoadAllScripts 	dom/workers/ScriptLoader.cpp:2116
25 	xul.dll 	mozilla::dom::workers::scriptloader::LoadMainScript(mozilla::dom::workers::WorkerPrivate*, nsAString_internal const&, mozilla::dom::workers::WorkerScriptType, mozilla::ErrorResult&) 	dom/workers/ScriptLoader.cpp:2232
26 	xul.dll 	`anonymous namespace'::CompileScriptRunnable::WorkerRun 	dom/workers/WorkerPrivate.cpp:518
27 	xul.dll 	mozilla::dom::workers::WorkerRunnable::Run() 	dom/workers/WorkerRunnable.cpp:375
28 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp:1067
29 	xul.dll 	NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/glue/nsThreadUtils.cpp:311
30 	xul.dll 	mozilla::dom::workers::WorkerPrivate::DoRunLoop(JSContext*) 	dom/workers/WorkerPrivate.cpp:4568
31 	xul.dll 	`anonymous namespace'::WorkerThreadPrimaryRunnable::Run 	dom/workers/RuntimeService.cpp:2861
32 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp:1067
33 	xul.dll 	NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/glue/nsThreadUtils.cpp:311
34 	xul.dll 	mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp:368
35 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc:225
36 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc:205
37 	xul.dll 	nsThread::ThreadFunc(void*) 	xpcom/threads/nsThread.cpp:465
38 	nss3.dll 	_PR_NativeRunThread 	nsprpub/pr/src/threads/combined/pruthr.c:397
39 	nss3.dll 	pr_root 	nsprpub/pr/src/md/windows/w95thred.c:95
40 	ucrtbase.dll 	_o__CIpow 	
41 	kernel32.dll 	BaseThreadInitThunk 	
42 	ntdll.dll 	__RtlUserThreadStart 	
43 	ntdll.dll 	_RtlUserThreadStart

this cross-platform crash signature has been around for a while, but spiked up in volume since 51.0b14, 52.0a2 build 20170113004016 and 53.0a1 build 20170113030227. 

this would be the pushlogs for those builds:
* https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=97d6f73643940256c0eb61e384c49bf6f6c49847&tochange=91f5293e9a89056565493ed5073c3842b0ee9fdc
* https://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?fromchange=1abb4b193740653f1161d86f38dbb79287e7d69b&tochange=1e322cd9c7410917d4c5f1fa5b2cd78eb4a79ec6
* https://hg.mozilla.org/releases/mozilla-beta/pushloghtml?fromchange=FIREFOX_51_0b13_RELEASE&tochange=FIREFOX_51_0b14_RELEASE

this is a very similar pattern of a regression-ranges like in bug 1331193, which makes me think those crashes might be related...

Comment 1

[Jan de Mooij [:jandem]]

This could be from bug 1325200; users running into the executable code limit we now have per process...

We could double the limit on beta for now, it's a bit less secure but should be low risk. In addition we could work on handling these "OOMs" more gracefully...

Comment 2

[Jan de Mooij [:jandem]]

Another low risk option is to check in JSRuntime::createJitRuntime that we have at least a few MB of executable memory available, and if not we just return OOM instead of creating a JitRuntime and potentially MOZ_CRASH'ing.

I think this will mostly affect power users with many tabs open. Unfortunately that information isn't on crash-stats.

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Patch

This bumps the limit on 32-bit from 128 MB to 160 MB, and on 64-bit from 512 MB to 640 MB.

Furthermore, when we are within 16 MB of this limit, we stop compiling Baseline/Ion/regexp/constructor code and in JSRuntime::createJitRuntime we will report OOM instead of calling jitRuntime_->initialize and potentially MOZ_CRASHing after that.

These heuristics should be sufficient to prevent the majority of crashes and this is very low risk.

Comment 4

[Gerry Chang [:gchang]]

Track 51+ as there is a spike in 51.0b14. Let's see if we can get this in 51 RC2.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3e2eddbfeaef
Avoid OOM crashes when we reach the executable code limit. r=luke

Comment 6

[[:philipp]]

thanks. do you think that bug 1331193 could be related or are the similar regression ranges just a coincidence?

Comment 7

[Jan de Mooij [:jandem]]

(In reply to [:philipp] from comment #6)
> thanks. do you think that bug 1331193 could be related or are the similar
> regression ranges just a coincidence?

I'm not familiar with that code, but I don't think these crashes are related. Maybe another patch landed and was backported around the same time?

Comment 8

[Jan de Mooij [:jandem]]

Attachment: Patch for Aurora and Beta

In bug 1325200 we added a limit on the amount of executable code we allow per process. As I mentioned there, this had some risk of causing OOMs and that's what's happening here :/

The patch here increases our limit quite a bit and this alone will make crashes less likely. Furthermore, if we get close to this limit, we now try harder to avoid OOM and crashing on OOM.

This patch should be very safe. It only affects cases where we allocate a lot of JIT code. That does happen (see the crashes here) but most users won't run into this I expect.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1325200.
[User impact if declined]: OOM crashes.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Not yet.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: See above.
[Why is the change risky/not risky?]: See above.
[String changes made/needed]: None.

Comment 9

[Jan de Mooij [:jandem]]

Attachment: Patch for ESR45

See comment 8.

Comment 10

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8827550 [details] [diff] [review]
Patch for Aurora and Beta

Given the possible increased likelihood of OOMs without this fix, I think it meets the criteria for inclusion in RC2 (got a second opinion from DVeditz). Let's uplift this to all affected branches once it's on m-central.

Comment 11

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8827553 [details] [diff] [review]
Patch for ESR45

Should be in ESR45.7

Comment 12

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Gerry, Liz, I am not aware of RC2 drivers at this point (but something always shows up). I approved this one for all branches assuming it's a good one to include in an RC2. Just FYI!

Comment 13

[Ryan VanderMeulen (PTO, back July 27)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7f15fe457dd3

Comment 14

[Ryan VanderMeulen (PTO, back July 27)]

https://hg.mozilla.org/releases/mozilla-beta/rev/5bf843087b55
https://hg.mozilla.org/releases/mozilla-release/rev/5bf843087b55

Comment 15

[Ryan VanderMeulen (PTO, back July 27)]

https://hg.mozilla.org/releases/mozilla-esr45/rev/f0329536734b

Comment 16

[Ryan VanderMeulen (PTO, back July 27)]

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

Comment 17

[Tino]

The error is not fixed. In the Nightly 22-01-2017. When trying to view a video on Facebook, the program was closed by marking an exception code c0000409 and with error in module "ucrtbase.dll".

Comment 18

[Emanuel Hoogeveen [:ehoogeveen]]

That's probably bug 1332905, which first appeared in yesterday's Nightly.

Comment 19

[Tino]

(In reply to Emanuel Hoogeveen [:ehoogeveen] from comment #18)
> That's probably bug 1332905, which first appeared in yesterday's Nightly.

Yes, it is very likely. Thank you.

Greetings.

Comment 20

[Mingzheng Ji]

Should this issue be reopened? The crash reports that has the same signature incease a lot after FF 59 release.