201829 2019-09-16 11:53:06 -0700 SamplingProfiler should hold API lock before reporting results 2019-09-16 13:49:26 -0700 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 tzagallo tzagallo commit-queue ews-watchlist keith_miller mark.lam msaboff saam webkit-bug-importer ysuzuki oldest_to_newest 1571194 0 tzagallo 2019-09-16 11:53:06 -0700 ... 1571214 1 378882 tzagallo 2019-09-16 12:55:14 -0700 Created attachment 378882 Patch 1571238 2 378882 ysuzuki 2019-09-16 13:27:58 -0700 Comment on attachment 378882 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378882&action=review r=me with nit. > Source/JavaScriptCore/runtime/SamplingProfiler.cpp:1029 > + JSLockHolder holder(m_vm); Can you ensure that this locking order (registerationLock -> APILock) is correct? (maybe, in practice, it does not matter). 1571245 3 tzagallo 2019-09-16 13:31:49 -0700 Thanks for the review! (In reply to Yusuke Suzuki from comment #2) > Can you ensure that this locking order (registerationLock -> APILock) is > correct? (maybe, in practice, it does not matter). The registrationLock is only guarding the append to the HashSet, `reportDataToOptionFile` is actually called later, from within the `atexit` lambda. 1571252 4 378882 commit-queue 2019-09-16 13:48:44 -0700 Comment on attachment 378882 Patch Clearing flags on attachment: 378882 Committed r249915: <https://trac.webkit.org/changeset/249915> 1571253 5 commit-queue 2019-09-16 13:48:46 -0700 All reviewed patches have been landed. Closing bug. 1571254 6 webkit-bug-importer 2019-09-16 13:49:26 -0700 <rdar://problem/55412906> 378882 2019-09-16 12:55:14 -0700 2019-09-16 13:48:44 -0700 Patch bug-201829-20190916125513.patch text/plain 1833 tzagallo U3VidmVyc2lvbiBSZXZpc2lvbjogMjQ5ODg2CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCA2 MDRkYmQ0YWUxODlmMDYyMGQ2N2I3YzlkZmMyZTg4NDc5ZGExZTQ0Li42NzliNGU2NGRhZDMxZDQz NmRiZjEyODgwMTczMjMzZDFjMmJkMjJhIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwyMCBAQAorMjAxOS0wOS0xNiAgVGFkZXUgWmFnYWxsbyAgPHR6YWdhbGxvQGFwcGxlLmNv bT4KKworICAgICAgICBTYW1wbGluZ1Byb2ZpbGVyIHNob3VsZCBob2xkIEFQSSBsb2NrIGJlZm9y ZSByZXBvcnRpbmcgcmVzdWx0cworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9MjAxODI5CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISku CisKKyAgICAgICAgUmlnaHQgbm93LCB0aGUgU2FtcGxpbmdQcm9maWxlciBjcmFzaGVzIGluIGRl YnVnIGJ1aWxkcyB3aGVuIHRyeWluZworICAgICAgICByZXBvcnQgcmVzdWx0cyBpZiBpdCBmaW5k cyBhIEpTRnVuY3Rpb24gb24gdGhlIHN0YWNrIHRoYXQgZG9lc24ndCBoYXZlCisgICAgICAgIFJh cmVEYXRhLiBJdCB0cmllcyB0byBhbGxvY2F0ZSB0aGUgZnVuY3Rpb24ncyByYXJlIGRhdGEgd2hl biB3ZSBjYWxsCisgICAgICAgIGdldE93blByb3BlcnR5U2xvdCBpbiBvcmRlciB0byBnZXQgdGhl IGZ1bmN0aW9uJ3MgbmFtZSwgYnV0IHRoYXQgZmFpbHMKKyAgICAgICAgYmVjYXVzZSB3ZSBhcmUg bm90IGhvbGRpbmcgdGhlIFZNJ3MgQVBJIGxvY2suIFdlIGZpeCBpdCBieSBqdXN0IGhvbGRpbmcK KyAgICAgICAgdGhlIGxvY2sgYmVmb3JlIHJlcG9ydGluZyB0aGUgcmVzdWx0cy4KKworICAgICAg ICAqIHJ1bnRpbWUvU2FtcGxpbmdQcm9maWxlci5jcHA6CisgICAgICAgIChKU0M6OlNhbXBsaW5n UHJvZmlsZXI6OnJlcG9ydERhdGFUb09wdGlvbkZpbGUpOgorCiAyMDE5LTA5LTE1ICBEYXZpZCBL aWx6ZXIgIDxkZGtpbHplckBhcHBsZS5jb20+CiAKICAgICAgICAgTGVhayBvZiBOU01hcFRhYmxl IGluIC1bSlNWaXJ0dWFsTWFjaGluZSBhZGRNYW5hZ2VkUmVmZXJlbmNlOndpdGhPd25lcjpdCmRp ZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9TYW1wbGluZ1Byb2ZpbGVy LmNwcCBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL1NhbXBsaW5nUHJvZmlsZXIuY3Bw CmluZGV4IDNkMTA2N2MzN2RiNzdiYmI4MjBjMDViNTBmODgzMzk1ODBiNzQwYWEuLjYxZmYzZDA3 ZjQwMmNjNzVlNDEzNjZkMTdhMmM5YzJjNDg0MTMwMWQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZh U2NyaXB0Q29yZS9ydW50aW1lL1NhbXBsaW5nUHJvZmlsZXIuY3BwCisrKyBiL1NvdXJjZS9KYXZh U2NyaXB0Q29yZS9ydW50aW1lL1NhbXBsaW5nUHJvZmlsZXIuY3BwCkBAIC0xMDI2LDYgKzEwMjYs NyBAQCB2b2lkIFNhbXBsaW5nUHJvZmlsZXI6OnJlcG9ydERhdGFUb09wdGlvbkZpbGUoKQogewog ICAgIGlmIChtX25lZWRzUmVwb3J0QXRFeGl0KSB7CiAgICAgICAgIG1fbmVlZHNSZXBvcnRBdEV4 aXQgPSBmYWxzZTsKKyAgICAgICAgSlNMb2NrSG9sZGVyIGhvbGRlcihtX3ZtKTsKICAgICAgICAg Y29uc3QgY2hhciogcGF0aCA9IE9wdGlvbnM6OnNhbXBsaW5nUHJvZmlsZXJQYXRoKCk7CiAgICAg ICAgIFN0cmluZ1ByaW50U3RyZWFtIHBhdGhPdXQ7CiAgICAgICAgIHBhdGhPdXQucHJpbnQocGF0 aCwgIi8iKTsK