JSGlobalContextRelease should be the last release of a JSContextGroupRef to make sure that Identifier table is that of the current virtual machine when that virtual machine is destroyed. In the current order, we save the Identifier table from the last JSGlobalData, load our table, deref the JSGlobalData (which is not destroyed because there's still a reference from the JSVirtualMachine object), then we restore the old Identifier table. When the JSVirtualMachine calls JSContextGroupRelease, the JSGlobalData will be destroyed with the incorrect Identifier table loaded in wtfThreadData(). We can either reorder these calls in JSContext's dealloc or make it so that JSContextGroupRelease also does the save/restore of the Identifier table like JSGlobalContextRelease.
This is a bug in C API. We need to add the Identifier table save/restore in JSContextGroupRelease.
<rdar://problem/13118874>
Created attachment 185578 [details] Patch
Committed r141321: <http://trac.webkit.org/changeset/141321>
+ JSLockHolder lock(globalData); I don't think this is valid -- you'll end up unlocking the lock after you've deallocated the JSGlobalData. I think you can just eliminate the lock and rely on thread-safe refcounting.
(In reply to comment #5) > + JSLockHolder lock(globalData); > > I don't think this is valid -- you'll end up unlocking the lock after you've deallocated the JSGlobalData. I think you can just eliminate the lock and rely on thread-safe refcounting. JSLockHolder refs the JSGlobalData when it locks it, so it will only be destroyed after the lock has released the JSGlobalData. I think you're right that we don't need the lock though. I was just trying to replicate what JSGlobalContextRelease does.
> JSLockHolder refs the JSGlobalData when it locks it, so it will only be destroyed after the lock has released the JSGlobalData. Cool cool.