442708 - JS_GC with GC_SET_SLOT_REQUEST doesn't loop until all threads are satisfied

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Brendan's patch

Brendan figured this one out. Basically one thread can call JS_GC with GC_SET_SLOT_REQUEST and, while running, another thread can do the same. The second thread will increment gclevel and exit early but will leave its setSlotRequest on the runtime. JS_GC didn't properly loop in that case and exited with the old ssr still set on the runtime.

The attached patch was written by Brendan and makes the only caller loop until it's slotrequest is serviced. It has fixed all the crashes I was seeing earlier with junk slotrequests left dangling on the runtime, r=me.

Comment 1

[Brendan Eich [:brendan]]

Ben, could you please land this? I'm juggling too many patches atm. It's not even in my tree (or mq). Thanks,

/be

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Sure thing.

Comment 3

[Johnny Stenback (:jst)]

Fix checked in.

Comment 4

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 327427 [details] [diff] [review]
Brendan's patch

Requesting approval, especially if bug 439765 lands.

Comment 5

[Daniel Veditz [:dveditz]]

What kind of crashes does this fix? prior to workers (not in 3.0.x) how are you getting multi-thread JS?

Comment 6

[Mike Connor [:mconnor]]

Comment on attachment 327427 [details] [diff] [review]
Brendan's patch

I think this is reasonably safe, and from the comments in the other bug can be triggered by other things, and has baked quite a bit to date.

Comment 7

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch for 1.9.0 branch

Comment 8

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 348375 [details] [diff] [review]
Patch for 1.9.0 branch

Wrong patch :(

Comment 9

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch for 1.9.0 branch

Comment 10

[Robert Sayre]

Checking in jsobj.c;
/cvsroot/mozilla/js/src/jsobj.c,v  <--  jsobj.c
new revision: 3.478; previous revision: 3.477
done