Objective-C API: JSContext's dealloc causes ASSERT due to ordering of releases

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Mark Hahnenberg]

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.

[Mark Hahnenberg]

This is a bug in C API. We need to add the Identifier table save/restore in JSContextGroupRelease.

[Radar WebKit Bug Importer]

<rdar://problem/13118874>

[Mark Hahnenberg]

Created attachment 185578 [details]
Patch

[Mark Hahnenberg]

Committed r141321: <http://trac.webkit.org/changeset/141321>

[Geoffrey Garen]

+        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.

[Mark Hahnenberg]

(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.

[Geoffrey Garen]

> JSLockHolder refs the JSGlobalData when it locks it, so it will only be destroyed after the lock has released the JSGlobalData.

Cool cool.