61669 2011-05-27 14:40:34 -0700 Ref BlobResourceHandle before calling doNotifyFinish() on main thread 2011-05-27 19:24:37 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 japhet japhet commit-queue jianli oldest_to_newest 411670 0 japhet 2011-05-27 14:40:34 -0700 In another patch, I changed WebCore timings slightly and found that we don't seem to be guaranteeing that a BlobResourceHandle will survive until doNotfyFinish() gets called on the appropriate thread. I haven't found a way to trigger this without modifying the loader rather substantially, but I do have a fix for the issue I saw. 411673 1 95217 japhet 2011-05-27 14:42:07 -0700 Created attachment 95217 patch #2 411675 2 95217 jianli 2011-05-27 14:52:37 -0700 Comment on attachment 95217 patch #2 View in context: https://bugs.webkit.org/attachment.cgi?id=95217&action=review > Source/WebCore/ChangeLog:5 > + ref() BlobResourceHandle before calling doNotifyFinish() The title is somewhat confusing to me. How about: Keep a reference to BlobResourceHandle before calling doNotifyFinish asynchronously to ensure it's still safe in the main thread. Which test covers this? Please point it out here. > Source/WebCore/platform/network/BlobResourceHandle.cpp:599 > + if (!handle->aborted()) We should say: if (!handle->aborted() && handle->client()) > Source/WebCore/platform/network/BlobResourceHandle.cpp:601 > + handle->deref(); Please add a comment for deref, like: Unbalance the deref in notifyFinish. > Source/WebCore/platform/network/BlobResourceHandle.cpp:607 > + ref(); Please add an empty line here. 411679 3 95220 japhet 2011-05-27 15:14:05 -0700 Created attachment 95220 Addressing comments 411682 4 95220 jianli 2011-05-27 15:17:04 -0700 Comment on attachment 95220 Addressing comments View in context: https://bugs.webkit.org/attachment.cgi?id=95220&action=review > Source/WebCore/ChangeLog:7 > + I triggered this crash in fast/files/file-reader-abort.html during a Please move this line and the line below to place after the bug link. 411683 5 95221 japhet 2011-05-27 15:18:33 -0700 Created attachment 95221 Moved test line in ChangeLog 411772 6 95221 commit-queue 2011-05-27 19:24:32 -0700 Comment on attachment 95221 Moved test line in ChangeLog Clearing flags on attachment: 95221 Committed r87595: <http://trac.webkit.org/changeset/87595> 411773 7 commit-queue 2011-05-27 19:24:37 -0700 All reviewed patches have been landed. Closing bug. 95217 2011-05-27 14:42:07 -0700 2011-05-27 15:14:05 -0700 patch #2 brh.txt text/plain 2163 japhet SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDg3NTczKQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTEtMDUtMjcgIE5hdGUgQ2hh cGluICA8amFwaGV0QGNocm9taXVtLm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkg KE9PUFMhKS4KKworICAgICAgICByZWYoKSBCbG9iUmVzb3VyY2VIYW5kbGUgYmVmb3JlIGNhbGxp bmcgZG9Ob3RpZnlGaW5pc2goKQorICAgICAgICB0byBlbnN1cmUgaXQncyBzdGlsbCBzYWZlIGlu IHRoZSBtYWluIHRocmVhZC4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTYxNjY5CisKKyAgICAgICAgKiBwbGF0Zm9ybS9uZXR3b3JrL0Jsb2JSZXNvdXJj ZUhhbmRsZS5jcHA6CisgICAgICAgIChXZWJDb3JlOjpkb05vdGlmeUZpbmlzaCk6CisgICAgICAg IChXZWJDb3JlOjpCbG9iUmVzb3VyY2VIYW5kbGU6Om5vdGlmeUZpbmlzaCk6CisgICAgICAgICog cGxhdGZvcm0vbmV0d29yay9CbG9iUmVzb3VyY2VIYW5kbGUuaDoKKyAgICAgICAgKFdlYkNvcmU6 OkJsb2JSZXNvdXJjZUhhbmRsZTo6YWJvcnRlZCk6CisKIDIwMTEtMDUtMjcgIEJyYWR5IEVpZHNv biAgPGJlaWRzb25AYXBwbGUuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IERhcmluIEFkbGVy LgpJbmRleDogU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vbmV0d29yay9CbG9iUmVzb3VyY2VIYW5k bGUuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL25ldHdvcmsvQmxv YlJlc291cmNlSGFuZGxlLmNwcAkocmV2aXNpb24gODc0NDUpCisrKyBTb3VyY2UvV2ViQ29yZS9w bGF0Zm9ybS9uZXR3b3JrL0Jsb2JSZXNvdXJjZUhhbmRsZS5jcHAJKHdvcmtpbmcgY29weSkKQEAg LTU5NiwxMiArNTk2LDE1IEBACiBzdGF0aWMgdm9pZCBkb05vdGlmeUZpbmlzaCh2b2lkKiBjb250 ZXh0KQogewogICAgIEJsb2JSZXNvdXJjZUhhbmRsZSogaGFuZGxlID0gc3RhdGljX2Nhc3Q8Qmxv YlJlc291cmNlSGFuZGxlKj4oY29udGV4dCk7Ci0gICAgaWYgKGhhbmRsZS0+Y2xpZW50KCkpCisg ICAgaWYgKCFoYW5kbGUtPmFib3J0ZWQoKSkKICAgICAgICAgaGFuZGxlLT5jbGllbnQoKS0+ZGlk RmluaXNoTG9hZGluZyhoYW5kbGUsIDApOworICAgIGhhbmRsZS0+ZGVyZWYoKTsKIH0KIAogdm9p ZCBCbG9iUmVzb3VyY2VIYW5kbGU6Om5vdGlmeUZpbmlzaCgpCiB7CisgICAgLy8gQmFsYW5jZWQg aW4gZG9Ob3RpZnlGaW5pc2goKS4KKyAgICByZWYoKTsKICAgICBpZiAobV9hc3luYykgewogICAg ICAgICAvLyBTY2hlZHVsZSB0byBub3RpZnkgdGhlIGNsaWVudCBmcm9tIGEgc3RhbmRhbG9uZSBm dW5jdGlvbiBiZWNhdXNlIHRoZSBjbGllbnQgbWlnaHQgZGlzcG9zZSB0aGUgaGFuZGxlIGltbWVk aWF0ZWx5IGZyb20gdGhlIGNhbGxiYWNrIGZ1bmN0aW9uCiAgICAgICAgIC8vIHdoaWxlIHdlIHN0 aWxsIGhhdmUgQmxvYlJlc291cmNlSGFuZGxlIGNhbGxzIGluIHRoZSBzdGFjay4KSW5kZXg6IFNv dXJjZS9XZWJDb3JlL3BsYXRmb3JtL25ldHdvcmsvQmxvYlJlc291cmNlSGFuZGxlLmgKPT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PQotLS0gU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vbmV0d29yay9CbG9iUmVzb3VyY2VIYW5k bGUuaAkocmV2aXNpb24gODc0NDUpCisrKyBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9uZXR3b3Jr L0Jsb2JSZXNvdXJjZUhhbmRsZS5oCSh3b3JraW5nIGNvcHkpCkBAIC02OCw2ICs2OCw4IEBACiAg ICAgdm9pZCBzdGFydCgpOwogICAgIGludCByZWFkU3luYyhjaGFyKiwgaW50KTsKIAorICAgIGJv b2wgYWJvcnRlZCgpIGNvbnN0IHsgcmV0dXJuIG1fYWJvcnRlZDsgfQorCiBwcml2YXRlOgogICAg IGZyaWVuZCB2b2lkIGRlbGF5ZWRTdGFydEJsb2JSZXNvdXJjZUhhbmRsZSh2b2lkKik7CiAK 95220 2011-05-27 15:14:05 -0700 2011-05-27 15:18:33 -0700 Addressing comments brh2.txt text/plain 2409 japhet SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDg3NTczKQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMjAgQEAKKzIwMTEtMDUtMjcgIE5hdGUgQ2hh cGluICA8amFwaGV0QGNocm9taXVtLm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkg KE9PUFMhKS4KKworICAgICAgICBLZWVwIGEgcmVmZXJlbmNlIHRvIEJsb2JSZXNvdXJjZUhhbmRs ZSBiZWZvcmUgY2FsbGluZyBkb05vdGlmeUZpbmlzaCgpCisgICAgICAgIGFzeW5jaHJvbm91c2x5 IHRvIGVuc3VyZSBpdCdzIHN0aWxsIHNhZmUgaW4gdGhlIG1haW4gdGhyZWFkLgorICAgICAgICBJ IHRyaWdnZXJlZCB0aGlzIGNyYXNoIGluIGZhc3QvZmlsZXMvZmlsZS1yZWFkZXItYWJvcnQuaHRt bCBkdXJpbmcgYQorICAgICAgICByZWZhY3RvciB0aGF0IGNoYW5nZWQgdGltaW5ncyBzbGlnaHRs eS4KKworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NjE2 NjkKKworICAgICAgICAqIHBsYXRmb3JtL25ldHdvcmsvQmxvYlJlc291cmNlSGFuZGxlLmNwcDoK KyAgICAgICAgKFdlYkNvcmU6OmRvTm90aWZ5RmluaXNoKToKKyAgICAgICAgKFdlYkNvcmU6OkJs b2JSZXNvdXJjZUhhbmRsZTo6bm90aWZ5RmluaXNoKToKKyAgICAgICAgKiBwbGF0Zm9ybS9uZXR3 b3JrL0Jsb2JSZXNvdXJjZUhhbmRsZS5oOgorICAgICAgICAoV2ViQ29yZTo6QmxvYlJlc291cmNl SGFuZGxlOjphYm9ydGVkKToKKwogMjAxMS0wNS0yNyAgQnJhZHkgRWlkc29uICA8YmVpZHNvbkBh cHBsZS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgRGFyaW4gQWRsZXIuCkluZGV4OiBTb3Vy Y2UvV2ViQ29yZS9wbGF0Zm9ybS9uZXR3b3JrL0Jsb2JSZXNvdXJjZUhhbmRsZS5jcHAKPT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PQotLS0gU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vbmV0d29yay9CbG9iUmVzb3VyY2VIYW5k bGUuY3BwCShyZXZpc2lvbiA4NzQ0NSkKKysrIFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL25ldHdv cmsvQmxvYlJlc291cmNlSGFuZGxlLmNwcAkod29ya2luZyBjb3B5KQpAQCAtNTk2LDEyICs1OTYs MTggQEAKIHN0YXRpYyB2b2lkIGRvTm90aWZ5RmluaXNoKHZvaWQqIGNvbnRleHQpCiB7CiAgICAg QmxvYlJlc291cmNlSGFuZGxlKiBoYW5kbGUgPSBzdGF0aWNfY2FzdDxCbG9iUmVzb3VyY2VIYW5k bGUqPihjb250ZXh0KTsKLSAgICBpZiAoaGFuZGxlLT5jbGllbnQoKSkKKyAgICBpZiAoIWhhbmRs ZS0+YWJvcnRlZCgpICYmIGhhbmRsZS0+Y2xpZW50KCkpCiAgICAgICAgIGhhbmRsZS0+Y2xpZW50 KCktPmRpZEZpbmlzaExvYWRpbmcoaGFuZGxlLCAwKTsKKworICAgIC8vIEJhbGFuY2UgdGhlIHJl ZigpIGluIEJsb2JSZXNvdXJjZUhhbmRsZTo6bm90ZnlGaW5pc2goKS4KKyAgICBoYW5kbGUtPmRl cmVmKCk7CiB9CiAKIHZvaWQgQmxvYlJlc291cmNlSGFuZGxlOjpub3RpZnlGaW5pc2goKQogewor ICAgIC8vIEJhbGFuY2VkIGluIGRvTm90aWZ5RmluaXNoKCkuCisgICAgcmVmKCk7CisKICAgICBp ZiAobV9hc3luYykgewogICAgICAgICAvLyBTY2hlZHVsZSB0byBub3RpZnkgdGhlIGNsaWVudCBm cm9tIGEgc3RhbmRhbG9uZSBmdW5jdGlvbiBiZWNhdXNlIHRoZSBjbGllbnQgbWlnaHQgZGlzcG9z ZSB0aGUgaGFuZGxlIGltbWVkaWF0ZWx5IGZyb20gdGhlIGNhbGxiYWNrIGZ1bmN0aW9uCiAgICAg ICAgIC8vIHdoaWxlIHdlIHN0aWxsIGhhdmUgQmxvYlJlc291cmNlSGFuZGxlIGNhbGxzIGluIHRo ZSBzdGFjay4KSW5kZXg6IFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL25ldHdvcmsvQmxvYlJlc291 cmNlSGFuZGxlLmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vbmV0d29y ay9CbG9iUmVzb3VyY2VIYW5kbGUuaAkocmV2aXNpb24gODc0NDUpCisrKyBTb3VyY2UvV2ViQ29y ZS9wbGF0Zm9ybS9uZXR3b3JrL0Jsb2JSZXNvdXJjZUhhbmRsZS5oCSh3b3JraW5nIGNvcHkpCkBA IC02OCw2ICs2OCw4IEBACiAgICAgdm9pZCBzdGFydCgpOwogICAgIGludCByZWFkU3luYyhjaGFy KiwgaW50KTsKIAorICAgIGJvb2wgYWJvcnRlZCgpIGNvbnN0IHsgcmV0dXJuIG1fYWJvcnRlZDsg fQorCiBwcml2YXRlOgogICAgIGZyaWVuZCB2b2lkIGRlbGF5ZWRTdGFydEJsb2JSZXNvdXJjZUhh bmRsZSh2b2lkKik7CiAK 95221 2011-05-27 15:18:33 -0700 2011-05-27 19:24:31 -0700 Moved test line in ChangeLog brh3.txt text/plain 2411 japhet SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDg3NTczKQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMjEgQEAKKzIwMTEtMDUtMjcgIE5hdGUgQ2hh cGluICA8amFwaGV0QGNocm9taXVtLm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkg KE9PUFMhKS4KKworICAgICAgICBLZWVwIGEgcmVmZXJlbmNlIHRvIEJsb2JSZXNvdXJjZUhhbmRs ZSBiZWZvcmUgY2FsbGluZyBkb05vdGlmeUZpbmlzaCgpCisgICAgICAgIGFzeW5jaHJvbm91c2x5 IHRvIGVuc3VyZSBpdCdzIHN0aWxsIHNhZmUgaW4gdGhlIG1haW4gdGhyZWFkLgorCisgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02MTY2OQorCisgICAgICAg IEkgdHJpZ2dlcmVkIHRoaXMgY3Jhc2ggaW4gZmFzdC9maWxlcy9maWxlLXJlYWRlci1hYm9ydC5o dG1sIGR1cmluZyBhCisgICAgICAgIHJlZmFjdG9yIHRoYXQgY2hhbmdlZCB0aW1pbmdzIHNsaWdo dGx5LgorCisgICAgICAgICogcGxhdGZvcm0vbmV0d29yay9CbG9iUmVzb3VyY2VIYW5kbGUuY3Bw OgorICAgICAgICAoV2ViQ29yZTo6ZG9Ob3RpZnlGaW5pc2gpOgorICAgICAgICAoV2ViQ29yZTo6 QmxvYlJlc291cmNlSGFuZGxlOjpub3RpZnlGaW5pc2gpOgorICAgICAgICAqIHBsYXRmb3JtL25l dHdvcmsvQmxvYlJlc291cmNlSGFuZGxlLmg6CisgICAgICAgIChXZWJDb3JlOjpCbG9iUmVzb3Vy Y2VIYW5kbGU6OmFib3J0ZWQpOgorCiAyMDExLTA1LTI3ICBCcmFkeSBFaWRzb24gIDxiZWlkc29u QGFwcGxlLmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBieSBEYXJpbiBBZGxlci4KSW5kZXg6IFNv dXJjZS9XZWJDb3JlL3BsYXRmb3JtL25ldHdvcmsvQmxvYlJlc291cmNlSGFuZGxlLmNwcAo9PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9uZXR3b3JrL0Jsb2JSZXNvdXJjZUhh bmRsZS5jcHAJKHJldmlzaW9uIDg3NDQ1KQorKysgU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vbmV0 d29yay9CbG9iUmVzb3VyY2VIYW5kbGUuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC01OTYsMTIgKzU5 NiwxOCBAQAogc3RhdGljIHZvaWQgZG9Ob3RpZnlGaW5pc2godm9pZCogY29udGV4dCkKIHsKICAg ICBCbG9iUmVzb3VyY2VIYW5kbGUqIGhhbmRsZSA9IHN0YXRpY19jYXN0PEJsb2JSZXNvdXJjZUhh bmRsZSo+KGNvbnRleHQpOwotICAgIGlmIChoYW5kbGUtPmNsaWVudCgpKQorICAgIGlmICghaGFu ZGxlLT5hYm9ydGVkKCkgJiYgaGFuZGxlLT5jbGllbnQoKSkKICAgICAgICAgaGFuZGxlLT5jbGll bnQoKS0+ZGlkRmluaXNoTG9hZGluZyhoYW5kbGUsIDApOworCisgICAgLy8gQmFsYW5jZSB0aGUg cmVmKCkgaW4gQmxvYlJlc291cmNlSGFuZGxlOjpub3RmeUZpbmlzaCgpLgorICAgIGhhbmRsZS0+ ZGVyZWYoKTsKIH0KIAogdm9pZCBCbG9iUmVzb3VyY2VIYW5kbGU6Om5vdGlmeUZpbmlzaCgpCiB7 CisgICAgLy8gQmFsYW5jZWQgaW4gZG9Ob3RpZnlGaW5pc2goKS4KKyAgICByZWYoKTsKKwogICAg IGlmIChtX2FzeW5jKSB7CiAgICAgICAgIC8vIFNjaGVkdWxlIHRvIG5vdGlmeSB0aGUgY2xpZW50 IGZyb20gYSBzdGFuZGFsb25lIGZ1bmN0aW9uIGJlY2F1c2UgdGhlIGNsaWVudCBtaWdodCBkaXNw b3NlIHRoZSBoYW5kbGUgaW1tZWRpYXRlbHkgZnJvbSB0aGUgY2FsbGJhY2sgZnVuY3Rpb24KICAg ICAgICAgLy8gd2hpbGUgd2Ugc3RpbGwgaGF2ZSBCbG9iUmVzb3VyY2VIYW5kbGUgY2FsbHMgaW4g dGhlIHN0YWNrLgpJbmRleDogU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vbmV0d29yay9CbG9iUmVz b3VyY2VIYW5kbGUuaAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9uZXR3 b3JrL0Jsb2JSZXNvdXJjZUhhbmRsZS5oCShyZXZpc2lvbiA4NzQ0NSkKKysrIFNvdXJjZS9XZWJD b3JlL3BsYXRmb3JtL25ldHdvcmsvQmxvYlJlc291cmNlSGFuZGxlLmgJKHdvcmtpbmcgY29weSkK QEAgLTY4LDYgKzY4LDggQEAKICAgICB2b2lkIHN0YXJ0KCk7CiAgICAgaW50IHJlYWRTeW5jKGNo YXIqLCBpbnQpOwogCisgICAgYm9vbCBhYm9ydGVkKCkgY29uc3QgeyByZXR1cm4gbV9hYm9ydGVk OyB9CisKIHByaXZhdGU6CiAgICAgZnJpZW5kIHZvaWQgZGVsYXllZFN0YXJ0QmxvYlJlc291cmNl SGFuZGxlKHZvaWQqKTsKIAo=