1313764 - "Attempt to acquire mutex GCLock with order 400 while holding FutexRuntime with order 500" in shell/futex.js

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Tooru Fujisawa [:arai]]

this is basically a dupe of bug 1313569, but I hit this permanently with async patch
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4134538044e9&selectedJob=30093498

here's simplified backtrace

#0 js`js::Mutex::lock at Mutex.cpp:54
#1 js`js::LockGuard<js::Mutex>::LockGuard at LockGuard.h:25
#2 js`js::LockGuard<js::Mutex>::LockGuard at LockGuard.h:24
#3 js`void mozilla::Maybe<js::LockGuard<js::Mutex> >::emplace<js::Mutex&> at Maybe.h:432
#4 js`js::AutoLockGC::lock at Runtime.h:1392
#5 js`js::AutoLockGC::AutoLockGC at Runtime.h:1383
#6 js`js::AutoLockGC::AutoLockGC at Runtime.h:1381
#7 js`js::gc::GCRuntime::collect at jsgc.cpp:6310
#8 js`js::gc::GCRuntime::gc at jsgc.cpp:6394
#9 js`js::gc::GCRuntime::runDebugGC at jsgc.cpp:6803
#10 js`js::gc::GCRuntime::gcIfNeededPerAllocation at Allocator.cpp:225
#11 js`bool js::gc::GCRuntime::checkAllocatorState< at Allocator.cpp:189
#12 js`js::Shape* js::Allocate<js::Shape,  at Allocator.cpp:139
#13 js`js::Shape::new_ at Shape-inl.h:86
#14 js`js::PropertyTree::getChild at jspropertytree.cpp:185
#15 js`js::NativeObject::getChildProperty at Shape.cpp:446
#16 js`js::NativeObject::addPropertyInternal at Shape.cpp:623
#17 js`js::NativeObject::addProperty at Shape.cpp:540
#18 js`js::NativeObject::addDataProperty at NativeObject.cpp:1027
#19 js`js::ErrorObject::assignInitialShape at ErrorObject.cpp:32
#20 js`bool js::EmptyShape::ensureInitialCustomShape<js::ErrorObject> at Shape-inl.h:121
#21 js`js::ErrorObject::init at ErrorObject.cpp:51
#22 js`js::ErrorObject::create at ErrorObject.cpp:112
#23 js`js::ErrorToException at jsexn.cpp:559
#24 js`ReportError at jscntxt.cpp:183
#25 js`js::ReportErrorNumberVA at jscntxt.cpp:693
#26 js`JS_ReportErrorNumberASCIIVA at jsapi.cpp:5608
#27 js`JS_ReportErrorNumberASCII at jsapi.cpp:5598
#28 js`js::FutexRuntime::wait at AtomicsObject.cpp:969
#29 js`js::atomics_wait at AtomicsObject.cpp:811
#30 js`js::CallJSNative at jscntxtinlines.h:239
#31 js`js::InternalCallOrConstruct at Interpreter.cpp:458
#32 js`InternalCall at Interpreter.cpp:503
#33 js`js::CallFromStack at Interpreter.cpp:509
#34 js`Interpret at Interpreter.cpp:2922
#35 js`js::RunScript at Interpreter.cpp:404
#36 js`js::InternalCallOrConstruct at Interpreter.cpp:476
#37 js`InternalCall at Interpreter.cpp:503
#38 js`js::Call at Interpreter.cpp:522
#39 js`JS_CallFunctionValue at jsapi.cpp:2768
#40 js`ShellInterruptCallback at js.cpp:558
#41 js`InvokeInterruptCallback at Runtime.cpp:541
#42 js`JSRuntime::handleInterrupt at Runtime.cpp:619
#43 js`js::FutexRuntime::wait at AtomicsObject.cpp:1053
#44 js`js::atomics_wait at AtomicsObject.cpp:811
#45 js`js::CallJSNative at jscntxtinlines.h:239
#46 js`js::InternalCallOrConstruct at Interpreter.cpp:458
#47 js`InternalCall at Interpreter.cpp:503
#48 js`js::CallFromStack at Interpreter.cpp:509
#49 js`Interpret at Interpreter.cpp:2922
#50 js`js::RunScript at Interpreter.cpp:404
#51 js`js::ExecuteKernel at Interpreter.cpp:685
#52 js`js::Execute at Interpreter.cpp:717
#53 js`ExecuteScript at jsapi.cpp:4309
#54 js`JS_ExecuteScript at jsapi.cpp:4342
#55 js`RunFile at js.cpp:650
#56 js`Process at js.cpp:1068
#57 js`ProcessArgs at js.cpp:7138
#58 js`Shell at js.cpp:7523
#59 js`main at js.cpp:7901
#60 js`start

it looks like GC happens in the interrupt handler called from js::FutexRuntime::wait, while futex expects no GC happens inside interrupt handler

>   _(GCLock,                      400) \
> ...
>   _(FutexRuntime,                500) \


I'm not sure how much this information is security-sensitive tho, marking as s-s just in case.

Comment 1

[Tooru Fujisawa [:arai]]

can you have a look?
also, does this or similar situation happens in browser?

Comment 2

[Tooru Fujisawa [:arai]]

so, the ordering was added by bug 1309909.
and it looks like the ordering conflicts with futex.

Comment 3

[Lars T Hansen [:lth]]

To the best of my knowledge there is no assumption in the futex code about not gc'ing in an interrupt, indeed the interrupt reenters the VM and gc should be normal.

I suspect that what's happening here that we need an UnlockGuard before the call to JS_ReportErrorNumberASCII, since it can apparently run the GC.  See the similar call below, before the call to handleInterrupt.

Probably not s-s.

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: Unlock mutex before calling JS_ReportErrorNumberASCII in js::FutexRuntime::wait.

Thanks!

indeed, adding UnlockGuard in the JS_ReportErrorNumberASCII's block solved the crash.

Comment 5

[Lars T Hansen [:lth]]

Comment on attachment 8805743 [details] [diff] [review]
Unlock mutex before calling JS_ReportErrorNumberASCII in js::FutexRuntime::wait.

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

Excellent, thank you!

Comment 6

[Tooru Fujisawa [:arai]]

Thank you for reviewing :)

so, this is not security-sensitive, but just implies the possibility of deadlock, according to bug 1309909 comment #0.
will land the patch shortly.

please someone open this up.

Comment 7

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a44d3d73a9903bc9e9a565fd7b96da30fecd782d
Bug 1313764 - Unlock mutex before calling JS_ReportErrorNumberASCII in js::FutexRuntime::wait. r=lth

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a44d3d73a990