65323 2011-07-28 10:17:11 -0700 REGRESSION: r91931 causes NOTREACHED to be hit via StorageTracker 2011-07-28 16:02:59 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 enne beidson beidson enne mjs oldest_to_newest 443540 0 enne 2011-07-28 10:17:11 -0700 After http://trac.webkit.org/changeset/91931/, a number of Chromium tests are hitting a NOTREACHED in debug mode. Stack trace from fast/js/global-constructors.html on Chromium Win Debug: SHOULD NEVER BE REACHED Backtrace: WebCore::SQLiteFileSystem::appendDatabaseFileNameToPath [0x0165E819+25] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\platform\sql\chromium\sqlitefilesystemchromium.cpp:68) WebCore::StorageTracker::trackerDatabasePath [0x0152D917+103] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\storage\storagetracker.cpp:100) WebCore::StorageTracker::openTrackerDatabase [0x0152DA01+193] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\storage\storagetracker.cpp:114) WebCore::StorageTracker::syncImportOriginIdentifiers [0x0152DD64+148] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\storage\storagetracker.cpp:159) WebCore::LocalStorageTask::performTask [0x0152AE05+117] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\storage\localstoragetask.cpp:94) WebCore::LocalStorageThread::threadEntryPoint [0x01528833+131] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\storage\localstoragethread.cpp:69) WebCore::LocalStorageThread::threadEntryPointCallback [0x0152879B+11] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\storage\localstoragethread.cpp:63) WTF::threadEntryPoint [0x020CA305+149] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\javascriptcore\wtf\threading.cpp:67) WTF::wtfThreadEntryPoint [0x008C18D9+89] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\javascriptcore\wtf\threadingwin.cpp:209) _callthreadstartex [0x00A7E783+83] (f:\dd\vctools\crt_bld\self_x86\crt\src\threadex.c:348) _threadstartex [0x00A7E724+164] (f:\dd\vctools\crt_bld\self_x86\crt\src\threadex.c:331) GetModuleFileNameA [0x7C80B729+442] Full set of tests: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&tests=fast%2Fjs%2Fglobal-constructors.html%2Cfast%2Floader%2Fchild-frame-add-after-back-forward.html%2Cfast%2Floader%2Fdocument-with-fragment-url-1.html%2Cfast%2Floader%2Fdocument-with-fragment-url-4.html%2Cfast%2Floader%2Fiframe-crash-on-missing-image.xhtml%2Cfast%2Floader%2Fjavascript-url-encoding-2.html%2Cfast%2Floader%2Fstateobjects%2Fdocument-destroyed-navigate-back-with-fragment-scroll.html%2Cfast%2Floader%2Fstateobjects%2Fpopstate-after-load-complete-body-attribute.html%2Cfast%2Floader%2Fstateobjects%2Fpushstate-clears-forward-history.html%2Cfast%2Floader%2Fstateobjects%2Freplacestate-updates-location.html%2Chttp%2Ftests%2Fappcache%2Flocal-content.html%2Chttp%2Ftests%2Fhistory%2Fback-to-post.php%2Chttp%2Ftests%2Finspector%2Fchange-iframe-src.html%2Chttp%2Ftests%2Finspector%2Fconsole-cd.html%2Chttp%2Ftests%2Finspector%2Fconsole-resource-errors.html%2Chttp%2Ftests%2Finspector%2Fconsole-websocket-error.html%2Chttp%2Ftests%2Finspector%2Fconsole-xhr-logging.html%2Chttp%2Ftests%2Finspector%2Finspect-iframe-from-different-domain.html%2Chttp%2Ftests%2Finspector%2Fnetwork-preflight-options.html%2Chttp%2Ftests%2Finspector%2Fresource-har-conversion.html 443558 1 enne 2011-07-28 10:49:49 -0700 This doesn't appear to happen in Safari, even in Debug builds. 443564 2 beidson 2011-07-28 11:16:15 -0700 Oh dear... It's not at all clear to me from this backtrace exactly what's going on - why 91931 caused this. I was under the impression that Chromium didn't make use of the StorageTracker (I remember when we were adding it there was a lot of hubbub about making sure it doesn't affect Chromium because they didn't want it) 91931 simple deferred StorageTracker initialization until the first time a storage area is accessed by the DOM. That code path is common and always has to happen on the main thread. From the background storage thread perspective, it is bizarre that there's any visible behavior change. 443565 3 beidson 2011-07-28 11:18:31 -0700 (In reply to comment #2) > Oh dear... > > It's not at all clear to me from this backtrace exactly what's going on - why 91931 caused this. I was under the impression that Chromium didn't make use of the StorageTracker (I remember when we were adding it there was a lot of hubbub about making sure it doesn't affect Chromium because they didn't want it) > > 91931 simple deferred StorageTracker initialization until the first time a storage area is accessed by the DOM. That code path is common and always has to happen on the main thread. > > From the background storage thread perspective, it is bizarre that there's any visible behavior change. Oh duh, I think I realized what's going on. By moving the initialization from WebKit to WebCore without platform guards, now *everyone* is getting an active, initialized StorageTracker. That was a pretty bad mistake on my part. My apologies - I'm working on a fix now. 443567 4 enne 2011-07-28 11:20:30 -0700 (In reply to comment #3) > That was a pretty bad mistake on my part. My apologies - I'm working on a fix now. Thanks for taking a look, even though this looked ostensibly like a Chromium-only problem. :) 443579 5 102274 beidson 2011-07-28 11:37:34 -0700 Created attachment 102274 Patch v1 - Flip the initialization flag to have "needs initialization" meaning, and only set it when ::initializeTracker() has been called (which Chromium doesn't do) 443611 6 beidson 2011-07-28 12:28:01 -0700 Okay http://trac.webkit.org/changeset/91943 should make this go away. Please close once you've seen things clear up on your tests. 443691 7 enne 2011-07-28 16:02:59 -0700 (In reply to comment #6) > Okay http://trac.webkit.org/changeset/91943 should make this go away. Please close once you've seen things clear up on your tests. Slow debug builders are slow. Finally, all green. Thanks again. :) 102274 2011-07-28 11:37:34 -0700 2011-07-28 12:25:53 -0700 Patch v1 - Flip the initialization flag to have "needs initialization" meaning, and only set it when ::initializeTracker() has been called (which Chromium doesn't do) patch.txt text/plain 2886 beidson SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDkxOTQwKQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMjEgQEAKKzIwMTEtMDctMjggIEJyYWR5IEVp ZHNvbiAgPGJlaWRzb25AYXBwbGUuY29tPgorCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQu b3JnL3Nob3dfYnVnLmNnaT9pZD02NTMyMworICAgICAgICByOTE5MzEgY2F1c2VzIE5PVFJFQUNI RUQgdG8gYmUgaGl0IHZpYSBTdG9yYWdlVHJhY2tlcgorCisgICAgICAgIENoYW5nZSB0aGUgbWVh bmluZyBvZiB0aGUgImhhcyBiZWVuIGluaXRpYWxpemVkIiBmbGFnIHRvICJuZWVkcyBpbml0aWFs aXphdGlvbiIsIGFuZCBvbmx5IHNldCBpdCB0byB0cnVlCisgICAgICAgIGlmIHRoZSA6OmluaXRp YWxpemVUcmFja2VyKCkgbWV0aG9kIGhhcyBiZWVuIGNhbGxlZC4KKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIHN0b3JhZ2UvU3RvcmFnZVRyYWNrZXIu Y3BwOgorICAgICAgICAoV2ViQ29yZTo6U3RvcmFnZVRyYWNrZXI6OmluaXRpYWxpemVUcmFja2Vy KTogU2V0IG1fbmVlZHNJbml0aWFsaXphdGlvbiB0byB0cnVlIHNpbmNlIHRoZSBjYWxsaW5nIFdl YktpdCBwb3J0IGV4cGVjdHMgZnVsbAorICAgICAgICAgIGluaXRpYWxpemF0aW9uIGluc3RlYWQg b2YgYSBkdW1teSB0cmFja2VyLgorICAgICAgICAoV2ViQ29yZTo6U3RvcmFnZVRyYWNrZXI6Omlu dGVybmFsSW5pdGlhbGl6ZSk6CisgICAgICAgIChXZWJDb3JlOjpTdG9yYWdlVHJhY2tlcjo6dHJh Y2tlcik6IE9ubHkgaW5pdGlhbGl6ZSB0aGUgdHJhY2tlciBpZiBpdCB3YXMgY3JlYXRlZCBpbiB0 aGUgYWJvdmUgaW5pdGlhbGl6ZVRyYWNrZXIoKS4KKyAgICAgICAgKFdlYkNvcmU6OlN0b3JhZ2VU cmFja2VyOjpTdG9yYWdlVHJhY2tlcik6CisgICAgICAgICogc3RvcmFnZS9TdG9yYWdlVHJhY2tl ci5oOgorCiAyMDExLTA3LTI4ICBEYXZpZCBLaWx6ZXIgIDxkZGtpbHplckBhcHBsZS5jb20+CiAK ICAgICAgICAgPGh0dHA6Ly93ZWJraXQub3JnL2IvNjUyODk+IFJlbW92ZSBHZW9sb2NhdGlvblBv c2l0aW9uQ2FjaGUKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL3N0b3JhZ2UvU3RvcmFnZVRyYWNrZXIu Y3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL3N0b3JhZ2UvU3RvcmFnZVRyYWNrZXIu Y3BwCShyZXZpc2lvbiA5MTkzMikKKysrIFNvdXJjZS9XZWJDb3JlL3N0b3JhZ2UvU3RvcmFnZVRy YWNrZXIuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC01Nyw2ICs1Nyw3IEBAIHZvaWQgU3RvcmFnZVRy YWNrZXI6OmluaXRpYWxpemVUcmFja2VyKGMKICAgICAgICAgc3RvcmFnZVRyYWNrZXIgPSBuZXcg U3RvcmFnZVRyYWNrZXIoc3RvcmFnZVBhdGgpOwogICAgIAogICAgIHN0b3JhZ2VUcmFja2VyLT5t X2NsaWVudCA9IGNsaWVudDsKKyAgICBzdG9yYWdlVHJhY2tlci0+bV9uZWVkc0luaXRpYWxpemF0 aW9uID0gdHJ1ZTsKIH0KIAogdm9pZCBTdG9yYWdlVHJhY2tlcjo6aW50ZXJuYWxJbml0aWFsaXpl KCkKQEAgLTcyLDE0ICs3MywxNCBAQCB2b2lkIFN0b3JhZ2VUcmFja2VyOjppbnRlcm5hbEluaXRp YWxpemUoCiAgICAgc3RvcmFnZVRyYWNrZXItPm1fdGhyZWFkLT5zdGFydCgpOyAgCiAgICAgc3Rv cmFnZVRyYWNrZXItPmltcG9ydE9yaWdpbklkZW50aWZpZXJzKCk7CiAgICAgCi0gICAgbV9pc0lu aXRpYWxpemVkID0gdHJ1ZTsKKyAgICBtX25lZWRzSW5pdGlhbGl6YXRpb24gPSBmYWxzZTsKIH0K IAogU3RvcmFnZVRyYWNrZXImIFN0b3JhZ2VUcmFja2VyOjp0cmFja2VyKCkKIHsKICAgICBpZiAo IXN0b3JhZ2VUcmFja2VyKQogICAgICAgICBzdG9yYWdlVHJhY2tlciA9IG5ldyBTdG9yYWdlVHJh Y2tlcigiIik7Ci0gICAgaWYgKCFzdG9yYWdlVHJhY2tlci0+bV9pc0luaXRpYWxpemVkKQorICAg IGlmIChzdG9yYWdlVHJhY2tlci0+bV9uZWVkc0luaXRpYWxpemF0aW9uKQogICAgICAgICBzdG9y YWdlVHJhY2tlci0+aW50ZXJuYWxJbml0aWFsaXplKCk7CiAgICAgCiAgICAgcmV0dXJuICpzdG9y YWdlVHJhY2tlcjsKQEAgLTkwLDcgKzkxLDcgQEAgU3RvcmFnZVRyYWNrZXI6OlN0b3JhZ2VUcmFj a2VyKGNvbnN0IFN0cgogICAgICwgbV9jbGllbnQoMCkKICAgICAsIG1fdGhyZWFkKExvY2FsU3Rv cmFnZVRocmVhZDo6Y3JlYXRlKCkpCiAgICAgLCBtX2lzQWN0aXZlKGZhbHNlKQotICAgICwgbV9p c0luaXRpYWxpemVkKGZhbHNlKQorICAgICwgbV9uZWVkc0luaXRpYWxpemF0aW9uKGZhbHNlKQog ewogfQogCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9zdG9yYWdlL1N0b3JhZ2VUcmFja2VyLmgKPT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvc3RvcmFnZS9TdG9yYWdlVHJhY2tlci5oCShyZXZp c2lvbiA5MTkzMikKKysrIFNvdXJjZS9XZWJDb3JlL3N0b3JhZ2UvU3RvcmFnZVRyYWNrZXIuaAko d29ya2luZyBjb3B5KQpAQCAtMTEyLDcgKzExMiw3IEBAIHByaXZhdGU6CiAgICAgT3duUHRyPExv Y2FsU3RvcmFnZVRocmVhZD4gbV90aHJlYWQ7CiAgICAgCiAgICAgYm9vbCBtX2lzQWN0aXZlOwot ICAgIGJvb2wgbV9pc0luaXRpYWxpemVkOworICAgIGJvb2wgbV9uZWVkc0luaXRpYWxpemF0aW9u OwogfTsKICAgICAKIH0gLy8gbmFtZXNwYWNlIFdlYkNvcmUK