1211939 - Assertion failure: js::CurrentThreadCanAccessRuntime(runtime_), at ../../dist/include/js/HeapAPI.h:135

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

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()>

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Tom S [:evilpie]]

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.

Comment 3

[Jan de Mooij [:jandem]]

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?

Comment 4

[Jon Coppeard (:jonco)]

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

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Patch

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.

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Patch

This is the right one actually.

Comment 7

[Jan de Mooij [:jandem]]

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 8

[Jon Coppeard (:jonco)]

Comment on attachment 8670702 [details] [diff] [review]
Patch

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

Yes, looks good.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0733455f9037

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0733455f9037