166778 2017-01-06 14:50:50 -0800 The ObjC API's JSVirtualMachine's map tables needs to be guarded by a lock. 2017-01-06 15:40:17 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Local Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 mark.lam mark.lam fpizlo ggaren jfbastien keith_miller msaboff saam webkit-bug-importer oldest_to_newest 1264524 0 mark.lam 2017-01-06 14:50:50 -0800 Now that we have a concurrent GC, access to JSVirtualMachine's m_externalObjectGraph and m_externalRememberedSet needs to guarded by a lock since both the GC marker thread and the mutator thread may access them at the same time. 1264526 1 mark.lam 2017-01-06 14:52:04 -0800 <rdar://problem/29759165> 1264529 2 mark.lam 2017-01-06 14:55:35 -0800 <rdar://problem/29761198> This is the correct radar. The previous one was a mistake. 1264536 3 298230 mark.lam 2017-01-06 15:04:29 -0800 Created attachment 298230 proposed patch. 1264539 4 298230 fpizlo 2017-01-06 15:16:29 -0800 Comment on attachment 298230 proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=298230&action=review LGTM > Source/JavaScriptCore/API/JSVirtualMachine.mm:160 > + std::lock_guard<Lock> lock(m_externalDataMutex); It's cool to use std::lock_guard, though I've been using: auto locker = holdLock(m_externalDataMutex); recently, since holdLock() is generic over the lock type without requiring you to say what the lock type is. I don't think it's a big deal, though. 1264555 5 mark.lam 2017-01-06 15:40:17 -0800 Thanks for the review. I've updated it to use holdLock instead. Patch landed in r210458: <http://trac.webkit.org/r210458>. 298230 2017-01-06 15:04:29 -0800 2017-01-06 15:16:29 -0800 proposed patch. bug-166778.patch text/plain 7150 mark.lam SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMjEwNDU1KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDI3IEBA CisyMDE3LTAxLTA2ICBNYXJrIExhbSAgPG1hcmsubGFtQGFwcGxlLmNvbT4KKworICAgICAgICBU aGUgT2JqQyBBUEkncyBKU1ZpcnR1YWxNYWNoaW5lJ3MgbWFwIHRhYmxlcyBuZWVkIHRvIGJlIGd1 YXJkZWQgYnkgYSBsb2NrLgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1 Zy5jZ2k/aWQ9MTY2Nzc4CisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS8yOTc2MTE5OD4KKworICAg ICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBOb3cgdGhhdCB3ZSBo YXZlIGEgY29uY3VycmVudCBHQywgYWNjZXNzIHRvIEpTVmlydHVhbE1hY2hpbmUncworICAgICAg ICBtX2V4dGVybmFsT2JqZWN0R3JhcGggYW5kIG1fZXh0ZXJuYWxSZW1lbWJlcmVkU2V0IG5lZWQg dG8gYmUgZ3VhcmRlZCBieSBhIGxvY2sKKyAgICAgICAgc2luY2UgYm90aCB0aGUgR0MgbWFya2Vy IHRocmVhZCBhbmQgdGhlIG11dGF0b3IgdGhyZWFkIG1heSBhY2Nlc3MgdGhlbSBhdCB0aGUKKyAg ICAgICAgc2FtZSB0aW1lLgorCisgICAgICAgICogQVBJL0pTVmlydHVhbE1hY2hpbmUubW06Cisg ICAgICAgICgtW0pTVmlydHVhbE1hY2hpbmUgYWRkRXh0ZXJuYWxSZW1lbWJlcmVkT2JqZWN0Ol0p OgorICAgICAgICAoLVtKU1ZpcnR1YWxNYWNoaW5lIGFkZE1hbmFnZWRSZWZlcmVuY2U6d2l0aE93 bmVyOl0pOgorICAgICAgICAoLVtKU1ZpcnR1YWxNYWNoaW5lIHJlbW92ZU1hbmFnZWRSZWZlcmVu Y2U6d2l0aE93bmVyOl0pOgorICAgICAgICAoLVtKU1ZpcnR1YWxNYWNoaW5lIGV4dGVybmFsRGF0 YU11dGV4XSk6CisgICAgICAgIChzY2FuRXh0ZXJuYWxPYmplY3RHcmFwaCk6CisgICAgICAgIChz Y2FuRXh0ZXJuYWxSZW1lbWJlcmVkU2V0KToKKworICAgICAgICAqIEFQSS9KU1ZpcnR1YWxNYWNo aW5lSW50ZXJuYWwuaDoKKyAgICAgICAgLSBEZWxldGVkIGV4dGVybmFsT2JqZWN0R3JhcGggbWV0 aG9kLiAgVGhlcmUncyBubyBuZWVkIHRvIGV4cG9zZSB0aGlzLgorCiAyMDE2LTEyLTMwICBGaWxp cCBQaXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CiAKICAgICAgICAgRGVmZXJHQzo6fkRlZmVyR0Mg c2hvdWxkIGJlIHN1cGVyIGNoZWFwCkluZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENvcmUvQVBJL0pT VmlydHVhbE1hY2hpbmUubW0KPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL0FQ SS9KU1ZpcnR1YWxNYWNoaW5lLm1tCShyZXZpc2lvbiAyMTA0NTUpCisrKyBTb3VyY2UvSmF2YVNj cmlwdENvcmUvQVBJL0pTVmlydHVhbE1hY2hpbmUubW0JKHdvcmtpbmcgY29weSkKQEAgLTEsNSAr MSw1IEBACiAvKgotICogQ29weXJpZ2h0IChDKSAyMDEzLTIwMTYgQXBwbGUgSW5jLiBBbGwgcmln aHRzIHJlc2VydmVkLgorICogQ29weXJpZ2h0IChDKSAyMDEzLTIwMTcgQXBwbGUgSW5jLiBBbGwg cmlnaHRzIHJlc2VydmVkLgogICoKICAqIFJlZGlzdHJpYnV0aW9uIGFuZCB1c2UgaW4gc291cmNl IGFuZCBiaW5hcnkgZm9ybXMsIHdpdGggb3Igd2l0aG91dAogICogbW9kaWZpY2F0aW9uLCBhcmUg cGVybWl0dGVkIHByb3ZpZGVkIHRoYXQgdGhlIGZvbGxvd2luZyBjb25kaXRpb25zCkBAIC04MSw2 ICs4MSw3IEBAICsgKEpTVmlydHVhbE1hY2hpbmUgKil3cmFwcGVyRm9ySlNDb250ZXgKIAogQGlt cGxlbWVudGF0aW9uIEpTVmlydHVhbE1hY2hpbmUgewogICAgIEpTQ29udGV4dEdyb3VwUmVmIG1f Z3JvdXA7CisgICAgTG9jayBtX2V4dGVybmFsRGF0YU11dGV4OwogICAgIE5TTWFwVGFibGUgKm1f Y29udGV4dENhY2hlOwogICAgIE5TTWFwVGFibGUgKm1fZXh0ZXJuYWxPYmplY3RHcmFwaDsKICAg ICBOU01hcFRhYmxlICptX2V4dGVybmFsUmVtZW1iZXJlZFNldDsKQEAgLTE1Niw2ICsxNTcsNyBA QCAtIChib29sKWlzT2xkRXh0ZXJuYWxPYmplY3Q6KGlkKW9iamVjdAogCiAtICh2b2lkKWFkZEV4 dGVybmFsUmVtZW1iZXJlZE9iamVjdDooaWQpb2JqZWN0CiB7CisgICAgc3RkOjpsb2NrX2d1YXJk PExvY2s+IGxvY2sobV9leHRlcm5hbERhdGFNdXRleCk7CiAgICAgQVNTRVJUKFtzZWxmIGlzT2xk RXh0ZXJuYWxPYmplY3Q6b2JqZWN0XSk7CiAgICAgW21fZXh0ZXJuYWxSZW1lbWJlcmVkU2V0IHNl dE9iamVjdDpAWUVTIGZvcktleTpvYmplY3RdOwogfQpAQCAtMTc1LDYgKzE3Nyw3IEBAIC0gKHZv aWQpYWRkTWFuYWdlZFJlZmVyZW5jZTooaWQpb2JqZWN0IHcKICAgICBpZiAoW3NlbGYgaXNPbGRF eHRlcm5hbE9iamVjdDpvd25lcl0gJiYgIVtzZWxmIGlzT2xkRXh0ZXJuYWxPYmplY3Q6b2JqZWN0 XSkKICAgICAgICAgW3NlbGYgYWRkRXh0ZXJuYWxSZW1lbWJlcmVkT2JqZWN0Om93bmVyXTsKICAK KyAgICBzdGQ6OmxvY2tfZ3VhcmQ8TG9jaz4gbG9jayhtX2V4dGVybmFsRGF0YU11dGV4KTsKICAg ICBOU01hcFRhYmxlICpvd25lZE9iamVjdHMgPSBbbV9leHRlcm5hbE9iamVjdEdyYXBoIG9iamVj dEZvcktleTpvd25lcl07CiAgICAgaWYgKCFvd25lZE9iamVjdHMpIHsKICAgICAgICAgTlNQb2lu dGVyRnVuY3Rpb25zT3B0aW9ucyB3ZWFrSURPcHRpb25zID0gTlNQb2ludGVyRnVuY3Rpb25zV2Vh a01lbW9yeSB8IE5TUG9pbnRlckZ1bmN0aW9uc09iamVjdFBlcnNvbmFsaXR5OwpAQCAtMjAyLDYg KzIwNSw3IEBAIC0gKHZvaWQpcmVtb3ZlTWFuYWdlZFJlZmVyZW5jZTooaWQpb2JqZWMKICAgICAK ICAgICBKU0M6OkpTTG9ja0hvbGRlciBsb2NrZXIodG9KUyhtX2dyb3VwKSk7CiAgICAgCisgICAg c3RkOjpsb2NrX2d1YXJkPExvY2s+IGxvY2sobV9leHRlcm5hbERhdGFNdXRleCk7CiAgICAgTlNN YXBUYWJsZSAqb3duZWRPYmplY3RzID0gW21fZXh0ZXJuYWxPYmplY3RHcmFwaCBvYmplY3RGb3JL ZXk6b3duZXJdOwogICAgIGlmICghb3duZWRPYmplY3RzKQogICAgICAgICByZXR1cm47CkBAIC0y NDgsNiArMjUyLDExIEBAIC0gKHZvaWQpYWRkQ29udGV4dDooSlNDb250ZXh0ICopd3JhcHBlciAK ICAgICBOU01hcEluc2VydChtX2NvbnRleHRDYWNoZSwgZ2xvYmFsQ29udGV4dCwgd3JhcHBlcik7 CiB9CiAKKy0gKExvY2smKWV4dGVybmFsRGF0YU11dGV4Cit7CisgICAgcmV0dXJuIG1fZXh0ZXJu YWxEYXRhTXV0ZXg7Cit9CisKIC0gKE5TTWFwVGFibGUgKilleHRlcm5hbE9iamVjdEdyYXBoCiB7 CiAgICAgcmV0dXJuIG1fZXh0ZXJuYWxPYmplY3RHcmFwaDsKQEAgLTI2MCwxMyArMjY5LDE0IEBA IC0gKE5TTWFwVGFibGUgKilleHRlcm5hbFJlbWVtYmVyZWRTZXQKIAogQGVuZAogCi12b2lkIHNj YW5FeHRlcm5hbE9iamVjdEdyYXBoKEpTQzo6Vk0mIHZtLCBKU0M6OlNsb3RWaXNpdG9yJiB2aXNp dG9yLCB2b2lkKiByb290KQorc3RhdGljIHZvaWQgc2NhbkV4dGVybmFsT2JqZWN0R3JhcGgoSlND OjpWTSYgdm0sIEpTQzo6U2xvdFZpc2l0b3ImIHZpc2l0b3IsIHZvaWQqIHJvb3QsIGJvb2wgbG9j a0FjcXVpcmVkKQogewogICAgIEBhdXRvcmVsZWFzZXBvb2wgewogICAgICAgICBKU1ZpcnR1YWxN YWNoaW5lICp2aXJ0dWFsTWFjaGluZSA9IFtKU1ZNV3JhcHBlckNhY2hlIHdyYXBwZXJGb3JKU0Nv bnRleHRHcm91cFJlZjp0b1JlZigmdm0pXTsKICAgICAgICAgaWYgKCF2aXJ0dWFsTWFjaGluZSkK ICAgICAgICAgICAgIHJldHVybjsKICAgICAgICAgTlNNYXBUYWJsZSAqZXh0ZXJuYWxPYmplY3RH cmFwaCA9IFt2aXJ0dWFsTWFjaGluZSBleHRlcm5hbE9iamVjdEdyYXBoXTsKKyAgICAgICAgTG9j ayYgZXh0ZXJuYWxEYXRhTXV0ZXggPSBbdmlydHVhbE1hY2hpbmUgZXh0ZXJuYWxEYXRhTXV0ZXhd OwogICAgICAgICBWZWN0b3I8dm9pZCo+IHN0YWNrOwogICAgICAgICBzdGFjay5hcHBlbmQocm9v dCk7CiAgICAgICAgIHdoaWxlICghc3RhY2suaXNFbXB0eSgpKSB7CkBAIC0yNzUsMjYgKzI4NSw0 NCBAQCB2b2lkIHNjYW5FeHRlcm5hbE9iamVjdEdyYXBoKEpTQzo6Vk0mIHZtCiAgICAgICAgICAg ICBpZiAodmlzaXRvci5jb250YWluc09wYXF1ZVJvb3RUcmlTdGF0ZShuZXh0Um9vdCkgPT0gVHJ1 ZVRyaVN0YXRlKQogICAgICAgICAgICAgICAgIGNvbnRpbnVlOwogICAgICAgICAgICAgdmlzaXRv ci5hZGRPcGFxdWVSb290KG5leHRSb290KTsKLSAgICAgICAgICAgIAotICAgICAgICAgICAgTlNN YXBUYWJsZSAqb3duZWRPYmplY3RzID0gW2V4dGVybmFsT2JqZWN0R3JhcGggb2JqZWN0Rm9yS2V5 OnN0YXRpY19jYXN0PGlkPihuZXh0Um9vdCldOwotICAgICAgICAgICAgZm9yIChpZCBvd25lZE9i amVjdCBpbiBvd25lZE9iamVjdHMpCi0gICAgICAgICAgICAgICAgc3RhY2suYXBwZW5kKHN0YXRp Y19jYXN0PHZvaWQqPihvd25lZE9iamVjdCkpOworCisgICAgICAgICAgICBhdXRvIGFwcGVuZE93 bmVkT2JqZWN0cyA9IFsmXSB7CisgICAgICAgICAgICAgICAgTlNNYXBUYWJsZSAqb3duZWRPYmpl Y3RzID0gW2V4dGVybmFsT2JqZWN0R3JhcGggb2JqZWN0Rm9yS2V5OnN0YXRpY19jYXN0PGlkPihu ZXh0Um9vdCldOworICAgICAgICAgICAgICAgIGZvciAoaWQgb3duZWRPYmplY3QgaW4gb3duZWRP YmplY3RzKQorICAgICAgICAgICAgICAgICAgICBzdGFjay5hcHBlbmQoc3RhdGljX2Nhc3Q8dm9p ZCo+KG93bmVkT2JqZWN0KSk7CisgICAgICAgICAgICB9OworCisgICAgICAgICAgICBpZiAobG9j a0FjcXVpcmVkKQorICAgICAgICAgICAgICAgIGFwcGVuZE93bmVkT2JqZWN0cygpOworICAgICAg ICAgICAgZWxzZSB7CisgICAgICAgICAgICAgICAgc3RkOjpsb2NrX2d1YXJkPExvY2s+IGxvY2so ZXh0ZXJuYWxEYXRhTXV0ZXgpOworICAgICAgICAgICAgICAgIGFwcGVuZE93bmVkT2JqZWN0cygp OworICAgICAgICAgICAgfQogICAgICAgICB9CiAgICAgfQogfQogCit2b2lkIHNjYW5FeHRlcm5h bE9iamVjdEdyYXBoKEpTQzo6Vk0mIHZtLCBKU0M6OlNsb3RWaXNpdG9yJiB2aXNpdG9yLCB2b2lk KiByb290KQoreworICAgIGJvb2wgbG9ja0FjcXVpcmVkID0gZmFsc2U7CisgICAgc2NhbkV4dGVy bmFsT2JqZWN0R3JhcGgodm0sIHZpc2l0b3IsIHJvb3QsIGxvY2tBY3F1aXJlZCk7Cit9CisKIHZv aWQgc2NhbkV4dGVybmFsUmVtZW1iZXJlZFNldChKU0M6OlZNJiB2bSwgSlNDOjpTbG90VmlzaXRv ciYgdmlzaXRvcikKIHsKICAgICBAYXV0b3JlbGVhc2Vwb29sIHsKICAgICAgICAgSlNWaXJ0dWFs TWFjaGluZSAqdmlydHVhbE1hY2hpbmUgPSBbSlNWTVdyYXBwZXJDYWNoZSB3cmFwcGVyRm9ySlND b250ZXh0R3JvdXBSZWY6dG9SZWYoJnZtKV07CiAgICAgICAgIGlmICghdmlydHVhbE1hY2hpbmUp CiAgICAgICAgICAgICByZXR1cm47CisgICAgICAgIExvY2smIGV4dGVybmFsRGF0YU11dGV4ID0g W3ZpcnR1YWxNYWNoaW5lIGV4dGVybmFsRGF0YU11dGV4XTsKKyAgICAgICAgc3RkOjpsb2NrX2d1 YXJkPExvY2s+IGxvY2soZXh0ZXJuYWxEYXRhTXV0ZXgpOwogICAgICAgICBOU01hcFRhYmxlICpl eHRlcm5hbE9iamVjdEdyYXBoID0gW3ZpcnR1YWxNYWNoaW5lIGV4dGVybmFsT2JqZWN0R3JhcGhd OwogICAgICAgICBOU01hcFRhYmxlICpleHRlcm5hbFJlbWVtYmVyZWRTZXQgPSBbdmlydHVhbE1h Y2hpbmUgZXh0ZXJuYWxSZW1lbWJlcmVkU2V0XTsKICAgICAgICAgZm9yIChpZCBrZXkgaW4gZXh0 ZXJuYWxSZW1lbWJlcmVkU2V0KSB7CiAgICAgICAgICAgICBOU01hcFRhYmxlICpvd25lZE9iamVj dHMgPSBbZXh0ZXJuYWxPYmplY3RHcmFwaCBvYmplY3RGb3JLZXk6a2V5XTsKKyAgICAgICAgICAg IGJvb2wgbG9ja0FjcXVpcmVkID0gdHJ1ZTsKICAgICAgICAgICAgIGZvciAoaWQgb3duZWRPYmpl Y3QgaW4gb3duZWRPYmplY3RzKQotICAgICAgICAgICAgICAgIHNjYW5FeHRlcm5hbE9iamVjdEdy YXBoKHZtLCB2aXNpdG9yLCBvd25lZE9iamVjdCk7CisgICAgICAgICAgICAgICAgc2NhbkV4dGVy bmFsT2JqZWN0R3JhcGgodm0sIHZpc2l0b3IsIG93bmVkT2JqZWN0LCBsb2NrQWNxdWlyZWQpOwog ICAgICAgICB9CiAgICAgICAgIFtleHRlcm5hbFJlbWVtYmVyZWRTZXQgcmVtb3ZlQWxsT2JqZWN0 c107CiAgICAgfQpJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL0FQSS9KU1ZpcnR1YWxNYWNo aW5lSW50ZXJuYWwuaAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlwdENvcmUvQVBJL0pT VmlydHVhbE1hY2hpbmVJbnRlcm5hbC5oCShyZXZpc2lvbiAyMTA0NTUpCisrKyBTb3VyY2UvSmF2 YVNjcmlwdENvcmUvQVBJL0pTVmlydHVhbE1hY2hpbmVJbnRlcm5hbC5oCSh3b3JraW5nIGNvcHkp CkBAIC0xLDUgKzEsNSBAQAogLyoKLSAqIENvcHlyaWdodCAoQykgMjAxMyBBcHBsZSBJbmMuIEFs bCByaWdodHMgcmVzZXJ2ZWQuCisgKiBDb3B5cmlnaHQgKEMpIDIwMTMsIDIwMTcgQXBwbGUgSW5j LiBBbGwgcmlnaHRzIHJlc2VydmVkLgogICoKICAqIFJlZGlzdHJpYnV0aW9uIGFuZCB1c2UgaW4g c291cmNlIGFuZCBiaW5hcnkgZm9ybXMsIHdpdGggb3Igd2l0aG91dAogICogbW9kaWZpY2F0aW9u LCBhcmUgcGVybWl0dGVkIHByb3ZpZGVkIHRoYXQgdGhlIGZvbGxvd2luZyBjb25kaXRpb25zCkBA IC00Nyw4ICs0Nyw2IEBAIEpTQ29udGV4dEdyb3VwUmVmIGdldEdyb3VwRnJvbVZpcnR1YWxNYWMK IC0gKEpTQ29udGV4dCAqKWNvbnRleHRGb3JHbG9iYWxDb250ZXh0UmVmOihKU0dsb2JhbENvbnRl eHRSZWYpZ2xvYmFsQ29udGV4dDsKIC0gKHZvaWQpYWRkQ29udGV4dDooSlNDb250ZXh0ICopd3Jh cHBlciBmb3JHbG9iYWxDb250ZXh0UmVmOihKU0dsb2JhbENvbnRleHRSZWYpZ2xvYmFsQ29udGV4 dDsKIAotLSAoTlNNYXBUYWJsZSAqKWV4dGVybmFsT2JqZWN0R3JhcGg7Ci0KIEBlbmQKICNlbmRp ZiAvLyBkZWZpbmVkKF9fT0JKQ19fKQogCg==