951957 - Assertion failure: isOwned(), at vm/Shape.h

Status: RESOLVED DUPLICATE of bug 938124

(Core :: JavaScript Engine: JIT, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: Stack from Crash Reporter

Assertion failure: isOwned(), at vm/Shape.h is an assertion that has hard-to-reproduce testcases, and which appeared fairly recently (probably in the past weeks?).

The changeset that landed this assert ( http://hg.mozilla.org/mozilla-central/annotate/04a70c8908de/js/src/vm/Shape.h#l705 ) was at:

http://hg.mozilla.org/mozilla-central/rev/d6d732ef5650

in bug 708735 by Waldo, but I'm not sure if it's the regressor.

JIT folks, any ideas? From the stack, are we jumping to/from the baseline/VM?

Comment 1

[Jeff Walden [:Waldo]]

Can't possibly be that -- the assertion was present in the method before that changeset, I just happened to modify the line that included it.

Comment 2

[Jesse Ruderman]

hg grep --all --follow 'slotSpan\(\) const \{ JS_ASSERT\(isOwned\(\)\)' js/src/vm/Shape.h

I think the assertion was added by:

changeset:   http://hg.mozilla.org/mozilla-central/rev/17bce205714e
user:        Brian Hackett
date:        Wed Oct 05 11:54:28 2011 -0700
summary:     Address shape number removal review comments, bug 684505.

Comment 3

[Brian Hackett [Laid off!]]

I think this was fixed by the most recent patch in bug 938124.  Looking at the crashing thread's stack, it is calling getSlotRef() on a global object in valueIsEval.  This isn't thread safe and if the main thread effects a shape change on the global object bad things can happen.  Namely, I think what happened here is the off thread read a shape pointer from the global object --- which is in dictionary mode, where only the last property owns its base shape --- and the main thread then reshaped the global and took away that shape's own base shape before the other thread could read its slot span, leading to this assertion.

When updating the patch in bug 938124 the thread safety analysis caught this bad use of getSlotRef(), and I replaced it with getSlotForCompilation(), which is safe to be called from the compilation thread.