906171 - triggerOperationCallback from within CodeGenerator::link causes deadlock

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

CodeGenerator::link takes the operation callback lock, then calls cx->malloc (via IonScript::New) which, onTooMuchMalloc, tries to trigger an operation callback.  This causes a deadlock b/c PRLocks are non-reentrant.  This is reproducible by running
  js --ion-parallel-compile=on awfy/benchmarks/asmjs-apps/bullet.js 2
which is what is hanging awfy on x64.

Here's the stack:
(gdb) bt<return> to continue, or q <return> to quit---
#0  0x00007ffff7bcb82c in __lll_lock_wait () from /lib/x86_64-linux-gnu/libpthread.so.0
#1  0x00007ffff7bc71b2 in _L_lock_1142 () from /lib/x86_64-linux-gnu/libpthread.so.0
#2  0x00007ffff7bc7130 in pthread_mutex_lock () from /lib/x86_64-linux-gnu/libpthread.so.0
#3  0x00007ffff79a2259 in PR_Lock () from /usr/lib/x86_64-linux-gnu/libnspr4.so
#4  0x000000000049dc0f in AutoLockForOperationCallback (rt=0x13ee5a0, this=<synthetic pointer>) at ../vm/Runtime.h:725
#5  JSRuntime::triggerOperationCallback (this=0x13ee5a0, trigger=JSRuntime::TriggerCallbackMainThread) at /moz/mi/js/src/vm/Runtime.cpp:529
#6  0x00000000006a2ed3 in updateMallocCounter (nbytes=65440, this=<optimized out>) at ../jscntxt.h:253
#7  malloc_ (bytes=65440, this=<optimized out>) at ../vm/Runtime.h:589
#8  js::ion::IonScript::New (cx=cx@entry=0x140cac0, frameSlots=192, frameSize=1536, snapshotsSize=61548, bailoutEntries=0, constants=146, safepointIndices=46, osiIndices=31, cacheEntries=0, runtimeSize=0, 
    safepointsSize=799, scriptEntries=1, callTargetEntries=0, backedgeEntries=8) at /moz/mi/js/src/jit/Ion.cpp:809
#9  0x0000000000697e6a in js::ion::CodeGenerator::link (this=this@entry=0x7fffcc30c440) at /moz/mi/js/src/jit/CodeGenerator.cpp:5611
#10 0x00000000006a551f in js::ion::AttachFinishedCompilations (cx=cx@entry=0x140cac0) at /moz/mi/js/src/jit/Ion.cpp:1554
#11 0x00000000005415e6 in js_InvokeOperationCallback (cx=0x140cac0) at /moz/mi/js/src/jscntxt.cpp:1023
#12 js_HandleExecutionInterrupt (cx=0x140cac0) at /moz/mi/js/src/jscntxt.cpp:1039

Comment 1

[Brian Hackett [Laid off!]]

Attachment: patch

Barf.  This patch rearranges CodeGenerator::link so IonScript::New is called before the callback lock is taken.  The only other fallible call in the method is IonLinker::newCode, which is still called with the lock held and can js_ReportOutOfMemory but doesn't seem to update any malloc counters or otherwise trigger the callback.

Comment 2

[Jan de Mooij [:jandem]]

Comment on attachment 791469 [details] [diff] [review]
patch

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

Good catch..

Comment 3

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/251540232d8f

Comment 4

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/251540232d8f