296006 - Assert (leading to 'crash' in debug build) when locking empty string

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Boris Zbarsky [:bzbarsky]]

BUILD: current trunk

STEPS TO REPRODUCE:
1)  Call JS_EvaluateUCScriptForPrincipals and wait for it to return
2)  Decide you need to do something that might trigger GC
3)  Try to lock the return value of JS_EvaluateUCScriptForPrincipals like so:

  if (ok && JSVAL_IS_GCTHING(val)) {
    ok = ::JS_LockGCThing(mContext, JSVAL_TO_GCTHING(val));

EXPECTED RESULTS:  keep |val| from being gc'ed

ACTUAL RESULTS:

#3  0xb7fd1477 in JS_Assert (s=0xb7ff0181 "JS_DHASH_ENTRY_IS_FREE(hdr)", 
    file=0xb7fefea0 "../../../mozilla/js/src/jsgc.c", ln=763)
    at ../../../mozilla/js/src/jsutil.c:155
#4  0xb7f70705 in js_LockGCThingRT (rt=0x80e3a70, thing=0x80ea798)
    at ../../../mozilla/js/src/jsgc.c:763

I this case, |val| is an empty JSString.  Shaver says we probably lock that at
startup, and the non-deep case in js_LockGCThingRT asserts if something is being
locked that's already locked.

Comment 1

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Attachment: Fix non-deep multiple-lock case

I thought about losing the optimization entirely, but it looks like this will
preserve it at minimal cost.

Comment 2

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Attachment: -w for review

Comment 3

[Boris Zbarsky [:bzbarsky]]

Without this my fix for bug 295983 is unhappy...

Comment 4

[Brendan Eich [:brendan]]

Man, I was tired.

But shaver, what's up with those crazy tabs?

/be

Comment 5

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

I was trying out XCode, which can be told "don't use tabs any more", but can't
apparently be told "banish the filthy tabs".  I'll M-x untabify before I commit,
natch. 

Comment 6

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Attachment: tabs are evil.

Sorry, I was learning to drive XCode.

Comment 7

[Brendan Eich [:brendan]]

Comment on attachment 184966 [details] [diff] [review]
tabs are evil.

>+        if (lhe->thing == 0) {

if (!lhe->thing), please, and r+a=me.  Thanks for wiping my old tired chin!

/be

Comment 8

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Fixed.