30920 2009-10-29 12:58:28 -0700 DOM Storage's condition variable needs to handle spurious wakeups 2009-10-29 16:22:54 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED P2 Normal --- 0 jorlow jorlow beidson fishd oldest_to_newest 159016 0 jorlow 2009-10-29 12:58:28 -0700 DOM Storage's condition variable needs to handle spurious wakeups. 159018 1 42127 jorlow 2009-10-29 12:59:49 -0700 Created attachment 42127 Patch v1 159025 2 42127 darin 2009-10-29 13:13:50 -0700 Comment on attachment 42127 Patch v1 Is there any way to test this? r=me 159028 3 42127 fishd 2009-10-29 13:15:25 -0700 Comment on attachment 42127 Patch v1 > Index: WebCore/ChangeLog ... > + Reviewed by NOBODY (OOPS!). > + > + DOM Storage's condition variable needs to handle spurious wakeups > + https://bugs.webkit.org/show_bug.cgi?id=30920 > + > + DOM Storage's condition variable needs to handle spurious wakeups. ^^^ repeating the summary? > Index: WebCore/storage/LocalStorageThread.cpp > =================================================================== > --- WebCore/storage/LocalStorageThread.cpp (revision 50240) > +++ WebCore/storage/LocalStorageThread.cpp (working copy) > @@ -112,7 +112,8 @@ void LocalStorageThread::terminate() > > m_queue.append(LocalStorageTask::createTerminate(this)); > > - m_terminateCondition.wait(m_terminateLock); > + while (!m_queue.isKilled()) > + m_terminateCondition.wait(m_terminateLock); nit: I think a more explicit 'bool m_terminated' member would be a bit nicer, purely so that one doesn't have to read the MessageQueue class to understand what is going on here. Also, I don't think that createTerminate needs to happen while m_terminateLock is held. 159033 4 jorlow 2009-10-29 13:17:38 -0700 (In reply to comment #2) > (From update of attachment 42127 [details]) > Is there any way to test this? I really wish there was, but I haven't been able to find a way to reliably reproduce the problem--inside or out of a layout test. Maybe if we had some sort of unit test framework I could induce it. Some day, I'd like to investigate how we could incorporate one into WebKit, but I'm pretty sure the answer today is no. :-( 159053 5 42134 jorlow 2009-10-29 13:45:23 -0700 Created attachment 42134 Patch v1 159139 6 jorlow 2009-10-29 16:22:54 -0700 Committed r50309: <http://trac.webkit.org/changeset/50309> 42127 2009-10-29 12:59:49 -0700 2009-10-29 13:45:15 -0700 Patch v1 bug-30920-20091029125947.patch text/plain 1209 jorlow SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1MDI5MikKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTUgQEAKKzIwMDktMTAtMjkgIEplcmVteSBPcmxvdyAgPGpvcmxvd0BjaHJvbWl1 bS5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAg RE9NIFN0b3JhZ2UncyBjb25kaXRpb24gdmFyaWFibGUgbmVlZHMgdG8gaGFuZGxlIHNwdXJpb3Vz IHdha2V1cHMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTMwOTIwCisKKyAgICAgICAgRE9NIFN0b3JhZ2UncyBjb25kaXRpb24gdmFyaWFibGUgbmVlZHMg dG8gaGFuZGxlIHNwdXJpb3VzIHdha2V1cHMuCisKKyAgICAgICAgKiBzdG9yYWdlL0xvY2FsU3Rv cmFnZVRocmVhZC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpMb2NhbFN0b3JhZ2VUaHJlYWQ6OnRl cm1pbmF0ZSk6CisKIDIwMDktMTAtMjkgIERpbWl0cmkgR2xhemtvdiAgPGRnbGF6a292QGNocm9t aXVtLm9yZz4KIAogICAgICAgICBVbnJldmlld2VkLCBidWlsZCBmaXguCkluZGV4OiBXZWJDb3Jl L3N0b3JhZ2UvTG9jYWxTdG9yYWdlVGhyZWFkLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL3N0 b3JhZ2UvTG9jYWxTdG9yYWdlVGhyZWFkLmNwcAkocmV2aXNpb24gNTAyNDApCisrKyBXZWJDb3Jl L3N0b3JhZ2UvTG9jYWxTdG9yYWdlVGhyZWFkLmNwcAkod29ya2luZyBjb3B5KQpAQCAtMTEyLDcg KzExMiw4IEBAIHZvaWQgTG9jYWxTdG9yYWdlVGhyZWFkOjp0ZXJtaW5hdGUoKQogCiAgICAgbV9x dWV1ZS5hcHBlbmQoTG9jYWxTdG9yYWdlVGFzazo6Y3JlYXRlVGVybWluYXRlKHRoaXMpKTsKIAot ICAgIG1fdGVybWluYXRlQ29uZGl0aW9uLndhaXQobV90ZXJtaW5hdGVMb2NrKTsKKyAgICB3aGls ZSAoIW1fcXVldWUuaXNLaWxsZWQoKSkKKyAgICAgICAgbV90ZXJtaW5hdGVDb25kaXRpb24ud2Fp dChtX3Rlcm1pbmF0ZUxvY2spOwogfQogCiB2b2lkIExvY2FsU3RvcmFnZVRocmVhZDo6cGVyZm9y bVRlcm1pbmF0ZSgp 42134 2009-10-29 13:45:23 -0700 2009-10-29 13:49:03 -0700 Patch v1 bug-30920-20091029134521.patch text/plain 2405 jorlow SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1MDI5OSkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMjAgQEAKKzIwMDktMTAtMjkgIEplcmVteSBPcmxvdyAgPGpvcmxvd0BjaHJvbWl1 bS5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAg RE9NIFN0b3JhZ2UncyBjb25kaXRpb24gdmFyaWFibGUgbmVlZHMgdG8gaGFuZGxlIHNwdXJpb3Vz IHdha2V1cHMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTMwOTIwCisKKyAgICAgICAgQWRkIGEgYm9vbGVhbiB0byBrZWVwIHRyYWNrIG9mIHdoZXRoZXIg aXQncyBiZWVuIHRlcm1pbmF0ZWQuICBDbGVhbgorICAgICAgICB1cCB0aGUgbG9ja2luZyBjb2Rl IGEgdGlueSBiaXQgdG8gbWFrZSBpdCBlYXNpZXIgdG8gcmVhZC4gIFRoZXJlJ3MKKyAgICAgICAg bm8gd2F5IHRvIHJlcHJvZHVjZSB0aGlzIHJlbGlhYmx5IGluIGEgTGF5b3V0VGVzdC4KKworICAg ICAgICAqIHN0b3JhZ2UvTG9jYWxTdG9yYWdlVGhyZWFkLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6 OkxvY2FsU3RvcmFnZVRocmVhZDo6TG9jYWxTdG9yYWdlVGhyZWFkKToKKyAgICAgICAgKFdlYkNv cmU6OkxvY2FsU3RvcmFnZVRocmVhZDo6dGVybWluYXRlKToKKyAgICAgICAgKFdlYkNvcmU6Okxv Y2FsU3RvcmFnZVRocmVhZDo6cGVyZm9ybVRlcm1pbmF0ZSk6CisgICAgICAgICogc3RvcmFnZS9M b2NhbFN0b3JhZ2VUaHJlYWQuaDoKKwogMjAwOS0xMC0yOSAgQWRhbSBCYXJ0aCAgPGFiYXJ0aEB3 ZWJraXQub3JnPgogCiAgICAgICAgIE5vIHJldmlldywgcm9sbGluZyBvdXQgcjUwMjk2LgpJbmRl eDogV2ViQ29yZS9zdG9yYWdlL0xvY2FsU3RvcmFnZVRocmVhZC5jcHAKPT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0g V2ViQ29yZS9zdG9yYWdlL0xvY2FsU3RvcmFnZVRocmVhZC5jcHAJKHJldmlzaW9uIDUwMjQwKQor KysgV2ViQ29yZS9zdG9yYWdlL0xvY2FsU3RvcmFnZVRocmVhZC5jcHAJKHdvcmtpbmcgY29weSkK QEAgLTQwLDYgKzQwLDcgQEAgUGFzc1JlZlB0cjxMb2NhbFN0b3JhZ2VUaHJlYWQ+IExvY2FsU3Rv cgogCiBMb2NhbFN0b3JhZ2VUaHJlYWQ6OkxvY2FsU3RvcmFnZVRocmVhZCgpCiAgICAgOiBtX3Ro cmVhZElEKDApCisgICAgLCBtX3Rlcm1pbmF0ZWQoZmFsc2UpCiB7CiAgICAgbV9zZWxmUmVmID0g dGhpczsKIH0KQEAgLTEwOCwxMSArMTA5LDExIEBAIHZvaWQgTG9jYWxTdG9yYWdlVGhyZWFkOjp0 ZXJtaW5hdGUoKQogICAgIGlmICghbV90aHJlYWRJRCkKICAgICAgICAgcmV0dXJuOwogCi0gICAg TXV0ZXhMb2NrZXIgbG9ja2VyKG1fdGVybWluYXRlTG9jayk7Ci0KICAgICBtX3F1ZXVlLmFwcGVu ZChMb2NhbFN0b3JhZ2VUYXNrOjpjcmVhdGVUZXJtaW5hdGUodGhpcykpOwogCi0gICAgbV90ZXJt aW5hdGVDb25kaXRpb24ud2FpdChtX3Rlcm1pbmF0ZUxvY2spOworICAgIE11dGV4TG9ja2VyIGxv Y2tlcihtX3Rlcm1pbmF0ZUxvY2spOworICAgIHdoaWxlICghbV90ZXJtaW5hdGVkKQorICAgICAg ICBtX3Rlcm1pbmF0ZUNvbmRpdGlvbi53YWl0KG1fdGVybWluYXRlTG9jayk7CiB9CiAKIHZvaWQg TG9jYWxTdG9yYWdlVGhyZWFkOjpwZXJmb3JtVGVybWluYXRlKCkKQEAgLTEyMiw2ICsxMjMsNyBA QCB2b2lkIExvY2FsU3RvcmFnZVRocmVhZDo6cGVyZm9ybVRlcm1pbmF0CiAgICAgbV9xdWV1ZS5r aWxsKCk7CiAKICAgICBNdXRleExvY2tlciBsb2NrZXIobV90ZXJtaW5hdGVMb2NrKTsKKyAgICBt X3Rlcm1pbmF0ZWQgPSB0cnVlOwogICAgIG1fdGVybWluYXRlQ29uZGl0aW9uLnNpZ25hbCgpOwog fQogCkluZGV4OiBXZWJDb3JlL3N0b3JhZ2UvTG9jYWxTdG9yYWdlVGhyZWFkLmgKPT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PQotLS0gV2ViQ29yZS9zdG9yYWdlL0xvY2FsU3RvcmFnZVRocmVhZC5oCShyZXZpc2lvbiA1MDI0 MCkKKysrIFdlYkNvcmUvc3RvcmFnZS9Mb2NhbFN0b3JhZ2VUaHJlYWQuaAkod29ya2luZyBjb3B5 KQpAQCAtNjcsNiArNjcsNyBAQCBuYW1lc3BhY2UgV2ViQ29yZSB7CiAKICAgICAgICAgTXV0ZXgg bV90ZXJtaW5hdGVMb2NrOwogICAgICAgICBUaHJlYWRDb25kaXRpb24gbV90ZXJtaW5hdGVDb25k aXRpb247CisgICAgICAgIGJvb2wgbV90ZXJtaW5hdGVkOwogICAgIH07CiAKIH0gLy8gbmFtZXNw YWNlIFdlYkNvcmU=