107978 2013-01-25 12:49:17 -0800 Objective-C API: JSContext's dealloc causes ASSERT due to ordering of releases 2013-01-30 21:08:34 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 mhahnenberg mhahnenberg barraclough ggaren webkit-bug-importer oldest_to_newest 816851 0 mhahnenberg 2013-01-25 12:49:17 -0800 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. 819364 1 mhahnenberg 2013-01-29 14:56:06 -0800 This is a bug in C API. We need to add the Identifier table save/restore in JSContextGroupRelease. 820557 2 webkit-bug-importer 2013-01-30 14:22:11 -0800 <rdar://problem/13118874> 820589 3 185578 mhahnenberg 2013-01-30 14:35:37 -0800 Created attachment 185578 Patch 820621 4 mhahnenberg 2013-01-30 14:48:24 -0800 Committed r141321: <http://trac.webkit.org/changeset/141321> 820668 5 ggaren 2013-01-30 15:16:17 -0800 + 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. 820966 6 mhahnenberg 2013-01-30 18:59:05 -0800 (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. 821050 7 ggaren 2013-01-30 21:08:34 -0800 > JSLockHolder refs the JSGlobalData when it locks it, so it will only be destroyed after the lock has released the JSGlobalData. Cool cool. 185578 2013-01-30 14:35:37 -0800 2013-01-30 14:42:39 -0800 Patch bug-107978-20130130143222.patch text/plain 1705 mhahnenberg SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTQxMzEyKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE2IEBA CisyMDEzLTAxLTMwICBNYXJrIEhhaG5lbmJlcmcgIDxtaGFobmVuYmVyZ0BhcHBsZS5jb20+CisK KyAgICAgICAgT2JqZWN0aXZlLUMgQVBJOiBKU0NvbnRleHQncyBkZWFsbG9jIGNhdXNlcyBBU1NF UlQgZHVlIHRvIG9yZGVyaW5nIG9mIHJlbGVhc2VzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMDc5NzgKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JP RFkgKE9PUFMhKS4KKworICAgICAgICBXZSBuZWVkIHRvIGFkZCB0aGUgSWRlbnRpZmllciB0YWJs ZSBzYXZlL3Jlc3RvcmUgaW4gSlNDb250ZXh0R3JvdXBSZWxlYXNlIHNvIHRoYXQgd2UgCisgICAg ICAgIGhhdmUgdGhlIGNvcnJlY3QgdGFibGUgaWYgd2UgZW5kIHVwIGRlc3Ryb3lpbmcgdGhlIEpT R2xvYmFsRGF0YS9IZWFwLgorCisgICAgICAgICogQVBJL0pTQ29udGV4dFJlZi5jcHA6CisgICAg ICAgIChKU0NvbnRleHRHcm91cFJlbGVhc2UpOgorCiAyMDEzLTAxLTMwICBNYXJrIEhhaG5lbmJl cmcgIDxtaGFobmVuYmVyZ0BhcHBsZS5jb20+CiAKICAgICAgICAgT2JqZWN0aXZlLUMgQVBJOiBl eGNlcHRpb25IYW5kbGVyIG5lZWRzIHRvIGJlIHJlbGVhc2VkIGluIEpTQ29udGV4dCBkZWFsbG9j CkluZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENvcmUvQVBJL0pTQ29udGV4dFJlZi5jcHAKPT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL0FQSS9KU0NvbnRleHRSZWYuY3BwCShyZXZp c2lvbiAxNDEzMDYpCisrKyBTb3VyY2UvSmF2YVNjcmlwdENvcmUvQVBJL0pTQ29udGV4dFJlZi5j cHAJKHdvcmtpbmcgY29weSkKQEAgLTY2LDcgKzY2LDE2IEBAIEpTQ29udGV4dEdyb3VwUmVmIEpT Q29udGV4dEdyb3VwUmV0YWluKEoKIAogdm9pZCBKU0NvbnRleHRHcm91cFJlbGVhc2UoSlNDb250 ZXh0R3JvdXBSZWYgZ3JvdXApCiB7Ci0gICAgdG9KUyhncm91cCktPmRlcmVmKCk7CisgICAgSWRl bnRpZmllclRhYmxlKiBzYXZlZElkZW50aWZpZXJUYWJsZTsKKyAgICBKU0dsb2JhbERhdGEmIGds b2JhbERhdGEgPSAqdG9KUyhncm91cCk7CisKKyAgICB7CisgICAgICAgIEpTTG9ja0hvbGRlciBs b2NrKGdsb2JhbERhdGEpOworICAgICAgICBzYXZlZElkZW50aWZpZXJUYWJsZSA9IHd0ZlRocmVh ZERhdGEoKS5zZXRDdXJyZW50SWRlbnRpZmllclRhYmxlKGdsb2JhbERhdGEuaWRlbnRpZmllclRh YmxlKTsKKyAgICAgICAgZ2xvYmFsRGF0YS5kZXJlZigpOworICAgIH0KKworICAgIHd0ZlRocmVh ZERhdGEoKS5zZXRDdXJyZW50SWRlbnRpZmllclRhYmxlKHNhdmVkSWRlbnRpZmllclRhYmxlKTsK IH0KIAogLy8gRnJvbSB0aGUgQVBJJ3MgcGVyc3BlY3RpdmUsIGEgZ2xvYmFsIGNvbnRleHQgcmVt YWlucyBhbGl2ZSBpZmYgaXQgaGFzIGJlZW4gSlNHbG9iYWxDb250ZXh0UmV0YWluZWQuCg==