Bugzilla@Mozilla – Bug 534051
Workers: Don't change the global object while GC is running
Last modified: 2010-08-14 01:35:59 PDT
Summon comment box
We're setting the global object on the context from another thread outside of a request. That's bad according to all sane peers, so we should stop. Also, I think we should check request depth inside JS_SetGlobalObject.
Created attachment 416977 [details] [review] Patch
Comment on attachment 416977 [details] [review] Patch OK. The JS change is a bit risky though. At a first glance I couldn't be sure that all the *other* places that call this function are doing it right.
Sorry--I meant to add: I'm OK with adding that CHECK_REQUEST if the tests pass. If they don't, that is if other code triggers the assertion, we need to look into it. People definitely shouldn't be racing with the GC to call that function. Shudder.
I pushed the patch to the tryserver
And tryserver loved it. Peterv and I think that this is the cause of bug 533000.
Comment on attachment 416977 [details] [review] Patch Over-the-shoulder r+sr=jst.
We need this on the branches, fixes GC crashes.
Pushed changeset 2000e14d36b2 to mozilla-central.
Comment on attachment 416977 [details] [review] Patch This patch should apply without any changes to 1.9.2 and 1.9.1.
This was blocking 1.9.2 (you accidentally removed the flag), so I'd go ahead and land there and ask jst to re-mark as blocking.
blocking, still! :)
Comment on attachment 416977 [details] [review] Patch Approved for 1.9.1.7, a=dveditz for release-drivers
Pushed to 192: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/f8e3e9f4d7c4
Pushed to 1.9.1 also. http://hg.mozilla.org/releases/mozilla-1.9.1/rev/620cfa6038af and http://hg.mozilla.org/releases/mozilla-1.9.1/rev/c3f393d2d171.
Ben, is there a way for QA to test this fix or do we have to trust hopefully existing tests?
This is guarded by a hard assertion so we'll see crash reports if anyone violates it.
But that means there no way for us to verify this fix manually. Is that correct?
Yes.