The following testcase crashes on mozilla-central-oom (https://github.com/nbp/gecko-dev/tree/oom) revision c119c16978b4f08f5e0c1269b52b9fdd9085be5f (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug, run with --fuzzing-safe --thread-count=2 --ion-check-range-analysis --ion-eager --baseline-eager --ion-extra-checks main.js): See attachment. Backtrace: Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7ffff5cab700 (LWP 9162)] 0x00000000004447fc in JS::shadow::Zone::runtimeFromMainThread (this=<optimized out>) at ../../dist/include/js/HeapAPI.h:135 #0 0x00000000004447fc in JS::shadow::Zone::runtimeFromMainThread (this=<optimized out>) at ../../dist/include/js/HeapAPI.h:135 #1 0x00000000007104a4 in runtimeFromMainThread (this=0x7ffff42b5800, this@entry=0x0) at js/src/gc/Zone.h:145 #2 JS::Zone::onOutOfMemory (this=this@entry=0x7ffff42b5800, allocFunc=allocFunc@entry=js::Malloc, nbytes=nbytes@entry=16, reallocPtr=reallocPtr@entry=0x0) at js/src/gc/Zone.h:145 #3 0x0000000000bbf8a7 in pod_malloc<unsigned char> (numElems=16, this=0x7ffff42b5800) at js/src/vm/MallocProvider.h:98 #4 js::SharedScriptData::new_ (cx=cx@entry=0x7ffff43916e0, codeLength=codeLength@entry=1, srcnotesLength=<optimized out>, natoms=natoms@entry=0) at js/src/jsscript.cpp:2313 #5 0x0000000000be7dc2 in JSScript::fullyInitFromEmitter (cx=0x7ffff43916e0, script=script@entry=..., bce=bce@entry=0x7ffff5caaa90) at js/src/jsscript.cpp:2824 #6 0x000000000062ecb1 in BytecodeCompiler::compileScript (this=this@entry=0x7ffff5ca9df0, scopeChain=..., scopeChain@entry=..., evalCaller=evalCaller@entry=...) at js/src/frontend/BytecodeCompiler.cpp:619 #7 0x000000000062ef2b in js::frontend::CompileScript (cx=<optimized out>, alloc=alloc@entry=0x7ffff69e3958, scopeChain=scopeChain@entry=..., enclosingStaticScope=..., enclosingStaticScope@entry=..., evalCaller=evalCaller@entry=..., options=..., srcBuf=..., source_=source_@entry=0x0, extraSct=extraSct@entry=0x0, sourceObjectOut=sourceObjectOut@entry=0x7ffff69e39c8) at js/src/frontend/BytecodeCompiler.cpp:808 #8 0x00000000006ecea9 in js::HelperThread::handleParseWorkload (this=this@entry=0x7ffff6933400) at js/src/vm/HelperThreads.cpp:1403 #9 0x00000000006ed97e in js::HelperThread::threadLoop (this=0x7ffff6933400) at js/src/vm/HelperThreads.cpp:1596 #10 0x00000000006c4aa1 in nspr::Thread::ThreadRoutine (arg=0x7ffff6931100) at js/src/vm/PosixNSPR.cpp:45 #11 0x00007ffff7bc4182 in start_thread (arg=0x7ffff5cab700) at pthread_create.c:312 #12 0x00007ffff6cb447d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111 rax 0x0 0 rbx 0x7ffff42b5800 140737289869312 rcx 0x7ffff6ca588d 140737333844109 rdx 0x0 0 rsi 0x7ffff6f7a9d0 140737336814032 rdi 0x7ffff6f791c0 140737336807872 rbp 0x7ffff5ca98a0 140737317083296 rsp 0x7ffff5ca98a0 140737317083296 r8 0x7ffff5cab700 140737317091072 r9 0x656363416e614364 7305792153200968548 r10 0x7ffff5ca9660 140737317082720 r11 0x7ffff6c27ee0 140737333329632 r12 0x0 0 r13 0x10 16 r14 0x0 0 r15 0x10 16 rip 0x4447fc <JS::shadow::Zone::runtimeFromMainThread() const+28> => 0x4447fc <JS::shadow::Zone::runtimeFromMainThread() const+28>: movl $0x87,0x0 0x444807 <JS::shadow::Zone::runtimeFromMainThread() const+39>: callq 0x4983a0 <abort()>

So js::SharedScriptData::new_ we call the Zone's pod_malloc, which OOMs. JS::Zone::onOutOfMemory is called as a result. runtimeFromMainThread is asserts because we are on the parsing thread. I am not sure what the right code here would look like.

ExclusiveContext::onOutOfMemory looks like this: void* onOutOfMemory(js::AllocFunction allocFunc, size_t nbytes, void* reallocPtr = nullptr) { return runtime_->onOutOfMemory(allocFunc, nbytes, reallocPtr, maybeJSContext()); } Question is if it's really okay to call JSRuntime::onOutOfMemory on the parse thread. This function calls GCRuntime::onOutOfMallocMemory(), I doubt that's safe to do on another thread. If ExclusiveContext::onOutOfMemory *is* safe, then we can use runtimeFromAnyThread in Zone::onOutOfMemory here, because this case is similar. If ExclusiveContext::onOutOfMemory is *not* safe, then JSRuntime::onOutOfMemory should have a CurrentThreadCanAccessRuntime assert or if-statement and maybe we should fix up ExclusiveContext::onOutOfMemory. Jon, what do you think?

I don't know. The safe thing to do is check CurrentThreadCanAccessRuntime() in Zone::onOutOfMemory() and return nullptr if it is false.

Don't call JSRuntime::onOutOfMemory if we're not on the main thread. Maybe JSRuntime::onOutOfMemory could use AutoLockForExclusiveAccess and it'd be safe, but that seems more complicated. I tried to move ExclusiveContext::onOutOfMemory to JSContext, but that's hard because MallocProvider<ExclusiveContext> calls it.

This is the right one actually.

Comment on attachment 8670702 [details] [diff] [review] Patch Review of attachment 8670702 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/gc/Zone.h @@ +143,5 @@ > > void* onOutOfMemory(js::AllocFunction allocFunc, size_t nbytes, void* reallocPtr = nullptr) { > + if (!CurrentThreadCanAccessRuntime(runtime_)) > + return nullptr; > + return runtimeFromAnyThread()->onOutOfMemory(allocFunc, nbytes, reallocPtr); This should stay as runtimeFromMainThread.

Comment on attachment 8670702 [details] [diff] [review] Patch Review of attachment 8670702 [details] [diff] [review]: ----------------------------------------------------------------- Yes, looks good.