1355573 - Assertion failure: !runtime->activeThreadHasExclusiveAccess

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[André Bargull [:anba]]

Test case:
---
function f(){};
Object.defineProperty(f, "name", {value: "a".repeat((1<<28)-1)});
print(f.bind().name.length);
print(("bound " + f.name).length);
---

Asserts with:
---
0x0000000000564dcc in js::AutoLockForExclusiveAccess::init (this=0x7fffffffa650, rt=0x7ffff696b000) at /home/andre/git/mozilla-central/js/src/jscntxt.h:1178
1178	            MOZ_ASSERT(!runtime->activeThreadHasExclusiveAccess);
---

Stacktrace:
---
#0  0x0000000000564dcc in js::AutoLockForExclusiveAccess::init(JSRuntime*) (this=0x7fffffffa650, rt=0x7ffff696b000) at /home/andre/git/mozilla-central/js/src/jscntxt.h:1178
#1  0x0000000000564e41 in js::AutoLockForExclusiveAccess::AutoLockForExclusiveAccess(JSContext*, mozilla::detail::GuardObjectNotifier&&) (this=0x7fffffffa650, cx=0x7ffff6975000, _notifier=<unknown type in /home/andre/git/mozilla-central/js/src/build-debug-master/js/src/shell/js, CU 0x60fc75, DIE 0x734ea8>) at /home/andre/git/mozilla-central/js/src/jscntxt.h:1188
#2  0x0000000000562191 in AtomizeAndCopyChars<unsigned char>(JSContext*, unsigned char const*, size_t, js::PinningBehavior) (cx=0x7ffff6975000, tbchars=0x7ffff69162a0 "/tmp/t.js", length=9, pin=js::DoNotPinAtom) at /home/andre/git/mozilla-central/js/src/jsatom.cpp:345
#3  0x00000000005629cf in js::Atomize(JSContext*, char const*, unsigned long, js::PinningBehavior) (cx=0x7ffff6975000, bytes=0x7ffff69162a0 "/tmp/t.js", length=9, pin=js::DoNotPinAtom)
    at /home/andre/git/mozilla-central/js/src/jsatom.cpp:442
#4  0x0000000000e67216 in js::SavedStacks::getLocation(JSContext*, js::FrameIter const&, JS::MutableHandle<js::SavedStacks::LocationValue>) (this=0x7ffff694e8f8, cx=0x7ffff6975000, iter=..., locationp=
  {source = 0x0, line = 0, column = 0}) at /home/andre/git/mozilla-central/js/src/vm/SavedStacks.cpp:1574
#5  0x0000000000e6603c in js::SavedStacks::insertFrames(JSContext*, js::FrameIter&, JS::MutableHandle<js::SavedFrame*>, mozilla::Variant<JS::AllFrames, JS::MaxFrames, JS::FirstSubsumedFrame>&&) (this=0x7ffff694e8f8, cx=0x7ffff6975000, iter=..., frame=0x0, capture=<unknown type in /home/andre/git/mozilla-central/js/src/build-debug-master/js/src/shell/js, CU 0x4170a9e, DIE 0x430f982>)
    at /home/andre/git/mozilla-central/js/src/vm/SavedStacks.cpp:1342
#6  0x0000000000e65714 in js::SavedStacks::saveCurrentStack(JSContext*, JS::MutableHandle<js::SavedFrame*>, mozilla::Variant<JS::AllFrames, JS::MaxFrames, JS::FirstSubsumedFrame>&&) (this=0x7ffff694e8f8, cx=0x7ffff6975000, frame=0x0, capture=<unknown type in /home/andre/git/mozilla-central/js/src/build-debug-master/js/src/shell/js, CU 0x4170a9e, DIE 0x430f24d>)
    at /home/andre/git/mozilla-central/js/src/vm/SavedStacks.cpp:1177
#7  0x0000000000b19d24 in JS::CaptureCurrentStack(JSContext*, JS::MutableHandle<JSObject*>, mozilla::Variant<JS::AllFrames, JS::MaxFrames, JS::FirstSubsumedFrame>&&) (cx=0x7ffff6975000, stackp=0x0, capture=<unknown type in /home/andre/git/mozilla-central/js/src/build-debug-master/js/src/shell/js, CU 0x2c230e0, DIE 0x2dcbf18>) at /home/andre/git/mozilla-central/js/src/jsapi.cpp:7084
#8  0x0000000000b2ac8b in CaptureStack(JSContext*, JS::MutableHandleObject) (cx=0x7ffff6975000, stack=0x0) at /home/andre/git/mozilla-central/js/src/jsexn.cpp:362
#9  0x0000000000b2c911 in js::ErrorToException(JSContext*, JSErrorReport*, JSErrorFormatString const* (*)(void*, unsigned int), void*) (cx=0x7ffff6975000, reportp=0x7fffffffb640, callback=0xb1d61d <js::GetErrorMessage(void*, unsigned int)>, userRef=0x0) at /home/andre/git/mozilla-central/js/src/jsexn.cpp:680
#10 0x0000000000b1b30e in ReportError(JSContext*, JSErrorReport*, JSErrorCallback, void*) (cx=0x7ffff6975000, reportp=0x7fffffffb640, callback=0xb1d61d <js::GetErrorMessage(void*, unsigned int)>, userRef=0x0) at /home/andre/git/mozilla-central/js/src/jscntxt.cpp:291
#11 0x0000000000b1c3e5 in js::ReportErrorNumberVA(JSContext*, unsigned int, JSErrorFormatString const* (*)(void*, unsigned int), void*, unsigned int, js::ErrorArgumentsType, __va_list_tag*) (cx=0x7ffff6975000, flags=0, callback=0xb1d61d <js::GetErrorMessage(void*, unsigned int)>, userRef=0x0, errorNumber=108, argumentsType=js::ArgumentsAreASCII, ap=0x7fffffffb720)
    at /home/andre/git/mozilla-central/js/src/jscntxt.cpp:890
#12 0x0000000000b156a8 in JS_ReportErrorNumberASCIIVA(JSContext*, JSErrorFormatString const* (*)(void*, unsigned int), void*, unsigned int, __va_list_tag*) (cx=0x7ffff6975000, errorCallback=0xb1d61d <js::GetErrorMessage(void*, unsigned int)>, userRef=0x0, errorNumber=108, ap=0x7fffffffb720) at /home/andre/git/mozilla-central/js/src/jsapi.cpp:5776
#13 0x0000000000b15642 in JS_ReportErrorNumberASCII(JSContext*, JSErrorFormatString const* (*)(void*, unsigned int), void*, unsigned int, ...) (cx=0x7ffff6975000, errorCallback=0xb1d61d <js::GetErrorMessage(void*, unsigned int)>, userRef=0x0, errorNumber=108) at /home/andre/git/mozilla-central/js/src/jsapi.cpp:5765
#14 0x0000000000b1b6da in js::ReportAllocationOverflow(JSContext*) (cx=0x7ffff6975000) at /home/andre/git/mozilla-central/js/src/jscntxt.cpp:403
#15 0x0000000000565523 in JSString::validateLength(JSContext*, unsigned long) (maybecx=0x7ffff6975000, length=268435461) at /home/andre/git/mozilla-central/js/src/vm/String-inl.h:100
#16 0x0000000000562318 in AtomizeAndCopyChars<unsigned char>(JSContext*, unsigned char const*, size_t, js::PinningBehavior) (cx=0x7ffff6975000, tbchars=0x7fffc2100000 "bound ", 'a' <repeats 194 times>..., length=268435461, pin=js::DoNotPinAtom) at /home/andre/git/mozilla-central/js/src/jsatom.cpp:358
#17 0x000000000056560c in js::AtomizeChars<unsigned char>(JSContext*, unsigned char const*, unsigned long, js::PinningBehavior) (cx=0x7ffff6975000, chars=0x7fffc2100000 "bound ", 'a' <repeats 194 times>..., length=268435461, pin=js::DoNotPinAtom) at /home/andre/git/mozilla-central/js/src/jsatom.cpp:450
#18 0x0000000000eca9b4 in js::StringBuffer::finishAtom() (this=0x7fffffffbb80) at /home/andre/git/mozilla-central/js/src/vm/StringBuffer.cpp:139
#19 0x0000000000e6f1b0 in intrinsic_FinishBoundFunctionInit(JSContext*, unsigned int, JS::Value*) (cx=0x7ffff6975000, argc=3, vp=0x7ffff4149178)
    at /home/andre/git/mozilla-central/js/src/vm/SelfHosting.cpp:530
...
---

Comment 1

[André Bargull [:anba]]

This could be related to bug 1350760:
In https://hg.mozilla.org/integration/mozilla-inbound/rev/f3a6587bc94a, the JSString::validateLength() call was moved, so we now have two callers with AutoLockForExclusiveAccess on the stack.

Comment 2

[Andrew McCreight [:mccr8]]

It sounds bad if multiple threads can write to this table at once.

Comment 3

[Sean Stangl [:sstangl]]

Setting needinfo? based on Bug 1350760.

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Patch

Call validateLength before taking the exclusive access lock.

Comment 5

[Jan de Mooij [:jandem]]

(In reply to Andrew McCreight [:mccr8] from comment #2)
> It sounds bad if multiple threads can write to this table at once.

Not multiple threads. A single thread is taking the lock twice, in opt builds this would result in a deadlock when there are helper thread zones present. If there are no helper thread zones, we don't take the lock and things should still be okay AFAIK.

Comment 6

[André Bargull [:anba]]

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

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

The test case may need too long to finish in non-opt builds (> 5 sec for me), is this an issue? Otherwise looks good to me!

Comment 7

[Andrew McCreight [:mccr8]]

Thanks for the explanation. I'll clear the sec rating. It sounds like this could be unhidden.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fe64a3b770fd
Fix AtomizeAndCopyChars to validate length before taking the lock as it may reenter. r=anba

Comment 9

[Jan de Mooij [:jandem]]

(In reply to André Bargull from comment #6)
> The test case may need too long to finish in non-opt builds (> 5 sec for
> me), is this an issue? Otherwise looks good to me!

I changed it to disable the test if getBuildConfiguration().debug is true. This way it at least runs in non-debug builds and the fuzzers can still reuse parts of it.

Comment 10

[Carsten Book [:Tomcat]]

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