201074 2019-08-23 05:31:04 -0700 CacheStorageConnection::computeRealBodySize is not thread-safe 2019-08-26 10:52:18 -0700 1 1 1 Unclassified WebKit Service Workers WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 youennf youennf achristensen cdumez commit-queue darin webkit-bug-importer oldest_to_newest 1564437 0 youennf 2019-08-23 05:31:04 -0700 CacheStorageConnection::computeRealBodySize is not thread-safe 1564438 1 377122 youennf 2019-08-23 05:34:07 -0700 Created attachment 377122 Patch 1564501 2 377122 darin 2019-08-23 10:40:12 -0700 Comment on attachment 377122 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377122&action=review > Source/WebCore/Modules/cache/CacheStorageConnection.cpp:48 > + uint64_t resultSize; > + BinarySemaphore semaphore; > + callOnMainThread([sessionID, formData = formData.isolatedCopy(), &semaphore, &resultSize] { > + resultSize = formData->lengthInBytes(sessionID); > + semaphore.signal(); > + }); > + semaphore.wait(); Why is the semaphore needed? Doesn’t callOnMainThread, which is synchronous, already contain enough synchronization to guarantee the correct value for resultSize? If not, it seems really hard to program with callOnMainThread correctly. 1564898 3 youennf 2019-08-26 01:55:17 -0700 (In reply to Darin Adler from comment #2) > Comment on attachment 377122 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=377122&action=review > > > Source/WebCore/Modules/cache/CacheStorageConnection.cpp:48 > > + uint64_t resultSize; > > + BinarySemaphore semaphore; > > + callOnMainThread([sessionID, formData = formData.isolatedCopy(), &semaphore, &resultSize] { > > + resultSize = formData->lengthInBytes(sessionID); > > + semaphore.signal(); > > + }); > > + semaphore.wait(); > > Why is the semaphore needed? Doesn’t callOnMainThread, which is synchronous, > already contain enough synchronization to guarantee the correct value for > resultSize? If not, it seems really hard to program with callOnMainThread > correctly. Using callOnMainThread does not wait, but I should replace the semaphore with a call to callOnMainThreadAndWait. 1564899 4 377236 youennf 2019-08-26 01:57:46 -0700 Created attachment 377236 Patch 1564959 5 377236 cdumez 2019-08-26 09:11:27 -0700 Comment on attachment 377236 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377236&action=review > Source/WebCore/Modules/cache/CacheStorageConnection.cpp:38 > + if (isMainThread()) This branch is not really needed, callOnMainThreadAndWait() already does the right thing for you. Or are you doing this as an optimization to avoid the isolatedCopy() of the FormData? I guess that's fine. 1564985 6 youennf 2019-08-26 10:13:27 -0700 (In reply to Chris Dumez from comment #5) > Comment on attachment 377236 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=377236&action=review > > > Source/WebCore/Modules/cache/CacheStorageConnection.cpp:38 > > + if (isMainThread()) > > This branch is not really needed, callOnMainThreadAndWait() already does the > right thing for you. Or are you doing this as an optimization to avoid the > isolatedCopy() of the FormData? I guess that's fine. Yes, there is no way we could optimise this. 1565008 7 377236 commit-queue 2019-08-26 10:51:24 -0700 Comment on attachment 377236 Patch Clearing flags on attachment: 377236 Committed r249105: <https://trac.webkit.org/changeset/249105> 1565009 8 commit-queue 2019-08-26 10:51:26 -0700 All reviewed patches have been landed. Closing bug. 1565010 9 webkit-bug-importer 2019-08-26 10:52:18 -0700 <rdar://problem/54713221> 377122 2019-08-23 05:34:07 -0700 2019-08-26 01:57:43 -0700 Patch bug-201074-20190823143405.patch text/plain 2641 youennf U3VidmVyc2lvbiBSZXZpc2lvbjogMjQ5MDQwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNzcwZWMwZjNhODI1OGQ4 YmVjNjE0ZWNjYTY3YjhmNmRmNDgyMDdmOC4uZjU2YWE2MmYxM2VmNGUxMmZhZTlmYjIxMzJlMTAx YTY1MzU0NTAzMyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDE5LTA4LTIzICBZb3Vl bm4gRmFibGV0ICA8eW91ZW5uQGFwcGxlLmNvbT4KKworICAgICAgICBDYWNoZVN0b3JhZ2VDb25u ZWN0aW9uOjpjb21wdXRlUmVhbEJvZHlTaXplIGlzIG5vdCB0aHJlYWQtc2FmZQorICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjAxMDc0CisKKyAgICAgICAg UmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgSW4gY2FzZSBvZiBhIGZvcm0g ZGF0YSwgdGhlIHNpemUgY29tcHV0YXRpb24gbWlnaHQgcmVxdWlyZSBzeW5jIElQQyB0byB0aGUg bmV0d29yayBwcm9jZXNzIHdoaWNoIGlzIG5vdCB0aHJlYWQtc2FmZQorICAgICAgICBJbiB0aGF0 IGNhc2UsIGhvcCB0byB0aGUgbWFpbiB0aHJlYWQgdG8gY29tcHV0ZSB0aGUgc2l6ZSBvZiB0aGUg Ym9keS4KKyAgICAgICAgQ292ZXJlZCBieSBleGlzdGluZyBzZXJ2aWNlIHdvcmtlciB0ZXN0cyBp biBEZWJ1ZyBtb2RlLgorCisgICAgICAgICogTW9kdWxlcy9jYWNoZS9DYWNoZVN0b3JhZ2VDb25u ZWN0aW9uLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OmZvcm1EYXRhU2l6ZSk6CisgICAgICAgIChX ZWJDb3JlOjpDYWNoZVN0b3JhZ2VDb25uZWN0aW9uOjpjb21wdXRlUmVhbEJvZHlTaXplKToKKwog MjAxOS0wOC0yMiAgRnVqaWkgSGlyb25vcmkgIDxIaXJvbm9yaS5GdWppaUBzb255LmNvbT4KIAog ICAgICAgICBbU1ZHXSAtd2Via2l0LWNsaXAtcGF0aCB0cmVhdHMgdXJsKGFiYyN4eXopIGFzIHVy bCgjeHl6KSBiZWNhdXNlIGl0IGNoZWNrcyBvbmx5IFVSTCBmcmFnbWVudCBwYXJ0CmRpZmYgLS1n aXQgYS9Tb3VyY2UvV2ViQ29yZS9Nb2R1bGVzL2NhY2hlL0NhY2hlU3RvcmFnZUNvbm5lY3Rpb24u Y3BwIGIvU291cmNlL1dlYkNvcmUvTW9kdWxlcy9jYWNoZS9DYWNoZVN0b3JhZ2VDb25uZWN0aW9u LmNwcAppbmRleCA0YzA0ZDI0Zjc0NWJmN2E0MjM3YzM4ZTU3YWRlYThlOGRkYTFiNDg1Li42Y2Y0 ODFlNDY1MTBiMGQ4YjgwZGYwNmNiZjNjMmMxZTE1M2I2ZjcxIDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViQ29yZS9Nb2R1bGVzL2NhY2hlL0NhY2hlU3RvcmFnZUNvbm5lY3Rpb24uY3BwCisrKyBiL1Nv dXJjZS9XZWJDb3JlL01vZHVsZXMvY2FjaGUvQ2FjaGVTdG9yYWdlQ29ubmVjdGlvbi5jcHAKQEAg LTI5LDE2ICsyOSwzMiBAQAogCiAjaW5jbHVkZSAiRmV0Y2hSZXNwb25zZS5oIgogI2luY2x1ZGUg PHd0Zi9SYW5kb21OdW1iZXIuaD4KKyNpbmNsdWRlIDx3dGYvdGhyZWFkcy9CaW5hcnlTZW1hcGhv cmUuaD4KIAogbmFtZXNwYWNlIFdlYkNvcmUgewogdXNpbmcgbmFtZXNwYWNlIFdlYkNvcmU6OkRP TUNhY2hlRW5naW5lOwogCitzdGF0aWMgaW5saW5lIHVpbnQ2NF90IGZvcm1EYXRhU2l6ZShjb25z dCBGb3JtRGF0YSYgZm9ybURhdGEsIFBBTDo6U2Vzc2lvbklEIHNlc3Npb25JRCkKK3sKKyAgICBp ZiAoaXNNYWluVGhyZWFkKCkpCisgICAgICAgIHJldHVybiBmb3JtRGF0YS5sZW5ndGhJbkJ5dGVz KHNlc3Npb25JRCk7CisKKyAgICB1aW50NjRfdCByZXN1bHRTaXplOworICAgIEJpbmFyeVNlbWFw aG9yZSBzZW1hcGhvcmU7CisgICAgY2FsbE9uTWFpblRocmVhZChbc2Vzc2lvbklELCBmb3JtRGF0 YSA9IGZvcm1EYXRhLmlzb2xhdGVkQ29weSgpLCAmc2VtYXBob3JlLCAmcmVzdWx0U2l6ZV0gewor ICAgICAgICByZXN1bHRTaXplID0gZm9ybURhdGEtPmxlbmd0aEluQnl0ZXMoc2Vzc2lvbklEKTsK KyAgICAgICAgc2VtYXBob3JlLnNpZ25hbCgpOworICAgIH0pOworICAgIHNlbWFwaG9yZS53YWl0 KCk7CisgICAgcmV0dXJuIHJlc3VsdFNpemU7Cit9CisKIHVpbnQ2NF90IENhY2hlU3RvcmFnZUNv bm5lY3Rpb246OmNvbXB1dGVSZWFsQm9keVNpemUoY29uc3QgRE9NQ2FjaGVFbmdpbmU6OlJlc3Bv bnNlQm9keSYgYm9keSkKIHsKICAgICB1aW50NjRfdCByZXN1bHQgPSAwOwotICAgIFdURjo6c3dp dGNoT24oYm9keSwgWyZdIChjb25zdCBSZWY8V2ViQ29yZTo6Rm9ybURhdGE+JiBmb3JtRGF0YSkg ewotICAgICAgICByZXN1bHQgPSBmb3JtRGF0YS0+bGVuZ3RoSW5CeXRlcyhzZXNzaW9uSUQoKSk7 Ci0gICAgfSwgWyZdIChjb25zdCBSZWY8V2ViQ29yZTo6U2hhcmVkQnVmZmVyPiYgYnVmZmVyKSB7 CisgICAgV1RGOjpzd2l0Y2hPbihib2R5LCBbJl0gKGNvbnN0IFJlZjxGb3JtRGF0YT4mIGZvcm1E YXRhKSB7CisgICAgICAgIHJlc3VsdCA9IGZvcm1EYXRhU2l6ZShmb3JtRGF0YSwgc2Vzc2lvbklE KCkpOworICAgIH0sIFsmXSAoY29uc3QgUmVmPFNoYXJlZEJ1ZmZlcj4mIGJ1ZmZlcikgewogICAg ICAgICByZXN1bHQgPSBidWZmZXItPnNpemUoKTsKICAgICB9LCBbXSAoY29uc3Qgc3RkOjpudWxs cHRyX3QmKSB7CiAgICAgfSk7Cg== 377236 2019-08-26 01:57:46 -0700 2019-08-26 10:51:24 -0700 Patch bug-201074-20190826105745.patch text/plain 2389 youennf U3VidmVyc2lvbiBSZXZpc2lvbjogMjQ5MDQwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNWYzZTE4MTI1NzdlZTZi YTI5ZDdjNDc3ZDI1OGJkZGE3Yzc2NDVjNy4uOTJmOTQzYWE4ZDMxYTA1MGRjYmJhMjQzZDI3MGZk ODg5YTY1OTNkYSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDE5LTA4LTI2ICBZb3Vl bm4gRmFibGV0ICA8eW91ZW5uQGFwcGxlLmNvbT4KKworICAgICAgICBDYWNoZVN0b3JhZ2VDb25u ZWN0aW9uOjpjb21wdXRlUmVhbEJvZHlTaXplIGlzIG5vdCB0aHJlYWQtc2FmZQorICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjAxMDc0CisKKyAgICAgICAg UmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgSW4gY2FzZSBvZiBhIGZvcm0g ZGF0YSwgdGhlIHNpemUgY29tcHV0YXRpb24gbWlnaHQgcmVxdWlyZSBzeW5jIElQQyB0byB0aGUg bmV0d29yayBwcm9jZXNzIHdoaWNoIGlzIG5vdCB0aHJlYWQtc2FmZQorICAgICAgICBJbiB0aGF0 IGNhc2UsIGhvcCB0byB0aGUgbWFpbiB0aHJlYWQgdG8gY29tcHV0ZSB0aGUgc2l6ZSBvZiB0aGUg Ym9keS4KKyAgICAgICAgQ292ZXJlZCBieSBleGlzdGluZyBzZXJ2aWNlIHdvcmtlciB0ZXN0cyBp biBEZWJ1ZyBtb2RlLgorCisgICAgICAgICogTW9kdWxlcy9jYWNoZS9DYWNoZVN0b3JhZ2VDb25u ZWN0aW9uLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OmZvcm1EYXRhU2l6ZSk6CisgICAgICAgIChX ZWJDb3JlOjpDYWNoZVN0b3JhZ2VDb25uZWN0aW9uOjpjb21wdXRlUmVhbEJvZHlTaXplKToKKwog MjAxOS0wOC0yMyAgWW91ZW5uIEZhYmxldCAgPHlvdWVubkBhcHBsZS5jb20+CiAKICAgICAgICAg UmVtb3ZlIE1lZGlhU3RyZWFtUHJpdmF0ZTo6c2NoZWR1bGVEZWZlcnJlZFRhc2sKZGlmZiAtLWdp dCBhL1NvdXJjZS9XZWJDb3JlL01vZHVsZXMvY2FjaGUvQ2FjaGVTdG9yYWdlQ29ubmVjdGlvbi5j cHAgYi9Tb3VyY2UvV2ViQ29yZS9Nb2R1bGVzL2NhY2hlL0NhY2hlU3RvcmFnZUNvbm5lY3Rpb24u Y3BwCmluZGV4IDRjMDRkMjRmNzQ1YmY3YTQyMzdjMzhlNTdhZGVhOGU4ZGRhMWI0ODUuLjlhYTY3 Yzc4NWNhODkwMDAwMDFjMmQ2ZGM4YzYyYTlmYzA0ZmFlZDIgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9X ZWJDb3JlL01vZHVsZXMvY2FjaGUvQ2FjaGVTdG9yYWdlQ29ubmVjdGlvbi5jcHAKKysrIGIvU291 cmNlL1dlYkNvcmUvTW9kdWxlcy9jYWNoZS9DYWNoZVN0b3JhZ2VDb25uZWN0aW9uLmNwcApAQCAt MzMsMTIgKzMzLDI0IEBACiBuYW1lc3BhY2UgV2ViQ29yZSB7CiB1c2luZyBuYW1lc3BhY2UgV2Vi Q29yZTo6RE9NQ2FjaGVFbmdpbmU7CiAKK3N0YXRpYyBpbmxpbmUgdWludDY0X3QgZm9ybURhdGFT aXplKGNvbnN0IEZvcm1EYXRhJiBmb3JtRGF0YSwgUEFMOjpTZXNzaW9uSUQgc2Vzc2lvbklEKQor eworICAgIGlmIChpc01haW5UaHJlYWQoKSkKKyAgICAgICAgcmV0dXJuIGZvcm1EYXRhLmxlbmd0 aEluQnl0ZXMoc2Vzc2lvbklEKTsKKworICAgIHVpbnQ2NF90IHJlc3VsdFNpemU7CisgICAgY2Fs bE9uTWFpblRocmVhZEFuZFdhaXQoW3Nlc3Npb25JRCwgZm9ybURhdGEgPSBmb3JtRGF0YS5pc29s YXRlZENvcHkoKSwgJnJlc3VsdFNpemVdIHsKKyAgICAgICAgcmVzdWx0U2l6ZSA9IGZvcm1EYXRh LT5sZW5ndGhJbkJ5dGVzKHNlc3Npb25JRCk7CisgICAgfSk7CisgICAgcmV0dXJuIHJlc3VsdFNp emU7Cit9CisKIHVpbnQ2NF90IENhY2hlU3RvcmFnZUNvbm5lY3Rpb246OmNvbXB1dGVSZWFsQm9k eVNpemUoY29uc3QgRE9NQ2FjaGVFbmdpbmU6OlJlc3BvbnNlQm9keSYgYm9keSkKIHsKICAgICB1 aW50NjRfdCByZXN1bHQgPSAwOwotICAgIFdURjo6c3dpdGNoT24oYm9keSwgWyZdIChjb25zdCBS ZWY8V2ViQ29yZTo6Rm9ybURhdGE+JiBmb3JtRGF0YSkgewotICAgICAgICByZXN1bHQgPSBmb3Jt RGF0YS0+bGVuZ3RoSW5CeXRlcyhzZXNzaW9uSUQoKSk7Ci0gICAgfSwgWyZdIChjb25zdCBSZWY8 V2ViQ29yZTo6U2hhcmVkQnVmZmVyPiYgYnVmZmVyKSB7CisgICAgV1RGOjpzd2l0Y2hPbihib2R5 LCBbJl0gKGNvbnN0IFJlZjxGb3JtRGF0YT4mIGZvcm1EYXRhKSB7CisgICAgICAgIHJlc3VsdCA9 IGZvcm1EYXRhU2l6ZShmb3JtRGF0YSwgc2Vzc2lvbklEKCkpOworICAgIH0sIFsmXSAoY29uc3Qg UmVmPFNoYXJlZEJ1ZmZlcj4mIGJ1ZmZlcikgewogICAgICAgICByZXN1bHQgPSBidWZmZXItPnNp emUoKTsKICAgICB9LCBbXSAoY29uc3Qgc3RkOjpudWxscHRyX3QmKSB7CiAgICAgfSk7Cg==