94899 2012-08-23 22:37:42 -0700 [V8] StringCache should not return already disposed string 2012-08-23 23:41:27 -0700 1 1 1 Unclassified WebKit WebCore JavaScript 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 haraken haraken abarth japhet webkit.review.bot oldest_to_newest 703754 0 haraken 2012-08-23 22:37:42 -0700 See this Chromium bug (http://code.google.com/p/chromium/issues/detail?id=143937) for details. I investigated the crash and found that v8ExternalString() can return already disposed strings: class StringCache { v8::Local<v8::String> v8ExternalString(StringImpl* stringImpl, v8::Isolate* isolate) { if (m_lastStringImpl.get() == stringImpl) { ASSERT(!m_lastV8String.IsNearDeath()); ASSERT(!m_lastV8String.IsEmpty()); return v8::Local<v8::String>::New(m_lastV8String); // m_lastV8String might be already Disposed. } return v8ExternalStringSlow(stringImpl, isolate); } } I couldn't find why m_lastV8String can be prematurely disposed, but the following fix will solve the crash: class StringCache { v8::Local<v8::String> v8ExternalString(StringImpl* stringImpl, v8::Isolate* isolate) { if (m_lastStringImpl.get() == stringImpl && m_lastV8String.IsWeak()) return v8::Local<v8::String>::New(m_lastV8String); return v8ExternalStringSlow(stringImpl, isolate); } } Although the ideal fix might be to fix the root cause of the premature disposal, I think that the above fix is reasonable for safety. In fact, we've so far encountered crashes caused by premature disposals (e.g. r123500). The above fix will prevent future crashes caused by premature disposals. 703758 1 160330 haraken 2012-08-23 22:42:57 -0700 Created attachment 160330 Patch 703773 2 160330 abarth 2012-08-23 22:57:43 -0700 Comment on attachment 160330 Patch Ok. This feels a bit like papering over the problem though. 703822 3 160330 webkit.review.bot 2012-08-23 23:41:24 -0700 Comment on attachment 160330 Patch Clearing flags on attachment: 160330 Committed r126547: <http://trac.webkit.org/changeset/126547> 703823 4 webkit.review.bot 2012-08-23 23:41:27 -0700 All reviewed patches have been landed. Closing bug. 160330 2012-08-23 22:42:57 -0700 2012-08-23 23:41:24 -0700 Patch bug-94899-20120824144253.patch text/plain 4176 haraken U3VidmVyc2lvbiBSZXZpc2lvbjogMTI2NTE3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggM2ZlZDBkZWNiNjc3ZTFl OWRiMDUzZDcxMWZhMTQxMjM5ZDlhOWI0NC4uNTkyYTAyMWY3Mzg0ZWE2NDE1ZWE5MDEyOTdlZTlk ODY2ZTIxZTAxZiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDU0IEBACisyMDEyLTA4LTIzICBLZW50 YXJvIEhhcmEgIDxoYXJha2VuQGNocm9taXVtLm9yZz4KKworICAgICAgICBbVjhdIFN0cmluZ0Nh Y2hlIHNob3VsZCBub3QgcmV0dXJuIGFscmVhZHkgZGlzcG9zZWQgc3RyaW5ncworICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9OTQ4OTkKKworICAgICAgICBS ZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBTZWUgdGhpcyBDaHJvbWl1bSBi dWcgKGh0dHA6Ly9jb2RlLmdvb2dsZS5jb20vcC9jaHJvbWl1bS9pc3N1ZXMvZGV0YWlsP2lkPTE0 MzkzNykKKyAgICAgICAgZm9yIGRldGFpbHMuCisKKyAgICAgICAgSSBpbnZlc3RpZ2F0ZWQgdGhl IGNyYXNoIGFuZCBmb3VuZCB0aGF0IHY4RXh0ZXJuYWxTdHJpbmcoKSBjYW4gcmV0dXJuCisgICAg ICAgIGFscmVhZHkgZGlzcG9zZWQgc3RyaW5nczoKKworICAgICAgICAgIGNsYXNzIFN0cmluZ0Nh Y2hlIHsKKyAgICAgICAgICAgIHY4OjpMb2NhbDx2ODo6U3RyaW5nPiB2OEV4dGVybmFsU3RyaW5n KFN0cmluZ0ltcGwqIHN0cmluZ0ltcGwsIC4uLikKKyAgICAgICAgICAgIHsKKyAgICAgICAgICAg ICAgICBpZiAobV9sYXN0U3RyaW5nSW1wbC5nZXQoKSA9PSBzdHJpbmdJbXBsKSB7CisgICAgICAg ICAgICAgICAgICAgIEFTU0VSVCghbV9sYXN0VjhTdHJpbmcuSXNOZWFyRGVhdGgoKSk7CisgICAg ICAgICAgICAgICAgICAgIEFTU0VSVCghbV9sYXN0VjhTdHJpbmcuSXNFbXB0eSgpKTsKKyAgICAg ICAgICAgICAgICAgICAgcmV0dXJuIHY4OjpMb2NhbDx2ODo6U3RyaW5nPjo6TmV3KG1fbGFzdFY4 U3RyaW5nKTsgLy8gbV9sYXN0VjhTdHJpbmcgbWlnaHQgYmUgYWxyZWFkeSBkaXNwb3NlZC4KKyAg ICAgICAgICAgICAgICB9CisgICAgICAgICAgICAgICAgcmV0dXJuIHY4RXh0ZXJuYWxTdHJpbmdT bG93KHN0cmluZ0ltcGwsIC4uLik7CisgICAgICAgICAgICB9CisgICAgICAgICAgfQorCisgICAg ICAgIEkgY291bGRuJ3QgZmluZCB3aHkgbV9sYXN0VjhTdHJpbmcgY2FuIGJlIHByZW1hdHVyZWx5 IGRpc3Bvc2VkLCBidXQgdGhlCisgICAgICAgIGZvbGxvd2luZyBmaXggc29sdmVzIHRoZSBjcmFz aDoKKworICAgICAgICAgIGNsYXNzIFN0cmluZ0NhY2hlIHsKKyAgICAgICAgICAgIHY4OjpMb2Nh bDx2ODo6U3RyaW5nPiB2OEV4dGVybmFsU3RyaW5nKFN0cmluZ0ltcGwqIHN0cmluZ0ltcGwsIC4u LikKKyAgICAgICAgICAgIHsKKyAgICAgICAgICAgICAgICBpZiAobV9sYXN0U3RyaW5nSW1wbC5n ZXQoKSA9PSBzdHJpbmdJbXBsICYmIG1fbGFzdFY4U3RyaW5nLklzV2VhaygpKQorICAgICAgICAg ICAgICAgICAgICByZXR1cm4gdjg6OkxvY2FsPHY4OjpTdHJpbmc+OjpOZXcobV9sYXN0VjhTdHJp bmcpOworICAgICAgICAgICAgICAgIHJldHVybiB2OEV4dGVybmFsU3RyaW5nU2xvdyhzdHJpbmdJ bXBsLCAuLi4pOworICAgICAgICAgICAgfQorICAgICAgICAgIH0KKworICAgICAgICBBbHRob3Vn aCB0aGUgaWRlYWwgZml4IG1pZ2h0IGJlIHRvIGZpeCB0aGUgcm9vdCBjYXVzZSBvZiB0aGUgcHJl bWF0dXJlIGRpc3Bvc2FsLAorICAgICAgICBJIHRoaW5rIHRoYXQgdGhlIHByb3Bvc2VkIGZpeCBp cyByZWFzb25hYmxlIGZvciBzYWZldHkuIEluIGZhY3QsIHdlJ3ZlIHNvIGZhcgorICAgICAgICBl bmNvdW50ZXJlZCBjcmFzaGVzIGNhdXNlZCBieSBwcmVtYXR1cmUgZGlzcG9zYWxzIChlLmcuIHIx MjM1MDApLiBUaGUgcHJvcG9zZWQgZml4CisgICAgICAgIHdpbGwgcHJldmVudCBmdXR1cmUgY3Jh c2hlcyBjYXVzZWQgYnkgcHJlbWF0dXJlIGRpc3Bvc2Fscy4KKworICAgICAgICBObyB0ZXN0cy4g VGhlIGNyYXNoIGRlcGVuZHMgb24gR0MgYmVoYXZpb3IgYW5kIEkgY291bGRuJ3Qgd3JpdGUgYSB0 ZXN0IHRoYXQKKyAgICAgICAgcmVwcm9kdWNlcyB0aGUgY3Jhc2guIE9wZW4gaHR0cDovL2xvcmUu Y29tL3Rlc3Rkcml2ZS9OYXZpZ2F0aW5nLXRoZS1Vbml2ZXJzZQorICAgICAgICBhbmQgY29uZmly bSB0aGF0IENocm9taXVtIGRvZXNuJ3QgY3Jhc2guCisKKyAgICAgICAgKiBiaW5kaW5ncy92OC9W OFZhbHVlQ2FjaGUuY3BwOgorICAgICAgICAoV2ViQ29yZTo6U3RyaW5nQ2FjaGU6OnY4RXh0ZXJu YWxTdHJpbmdTbG93KToKKyAgICAgICAgKiBiaW5kaW5ncy92OC9WOFZhbHVlQ2FjaGUuaDoKKyAg ICAgICAgKFdlYkNvcmU6OlN0cmluZ0NhY2hlOjp2OEV4dGVybmFsU3RyaW5nKToKKwogMjAxMi0w OC0yMyAgQWRhbSBCYXJ0aCAgPGFiYXJ0aEB3ZWJraXQub3JnPgogCiAgICAgICAgIFVucmV2aWV3 ZWQgYXR0ZW1wdCB0byBmaXggYnVpbGQgZmFpbHVyZSBpbiBEZWJ1Zy4KZGlmZiAtLWdpdCBhL1Nv dXJjZS9XZWJDb3JlL2JpbmRpbmdzL3Y4L1Y4VmFsdWVDYWNoZS5jcHAgYi9Tb3VyY2UvV2ViQ29y ZS9iaW5kaW5ncy92OC9WOFZhbHVlQ2FjaGUuY3BwCmluZGV4IDdkZTQzMTg5MzIzYzRhZTc4Yzhl ZDUwMWI5ZDAzNzJkZTg4ODI2MjUuLjFjMjUxMDY2NDNlMDc5MGNkOGM2YTRmZTFlYjkwOWFlOGQ0 N2Y5MjYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL3Y4L1Y4VmFsdWVDYWNo ZS5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvYmluZGluZ3MvdjgvVjhWYWx1ZUNhY2hlLmNwcApA QCAtNjksNyArNjksNyBAQCB2ODo6TG9jYWw8djg6OlN0cmluZz4gU3RyaW5nQ2FjaGU6OnY4RXh0 ZXJuYWxTdHJpbmdTbG93KFN0cmluZ0ltcGwqIHN0cmluZ0ltcGwsCiAgICAgdjg6OlN0cmluZyog Y2FjaGVkVjhTdHJpbmcgPSBtX3N0cmluZ0NhY2hlLmdldChzdHJpbmdJbXBsKTsKICAgICBpZiAo Y2FjaGVkVjhTdHJpbmcpIHsKICAgICAgICAgdjg6OlBlcnNpc3RlbnQ8djg6OlN0cmluZz4gaGFu ZGxlKGNhY2hlZFY4U3RyaW5nKTsKLSAgICAgICAgaWYgKCFoYW5kbGUuSXNOZWFyRGVhdGgoKSAm JiAhaGFuZGxlLklzRW1wdHkoKSkgeworICAgICAgICBpZiAoaGFuZGxlLklzV2VhaygpKSB7CiAg ICAgICAgICAgICBtX2xhc3RTdHJpbmdJbXBsID0gc3RyaW5nSW1wbDsKICAgICAgICAgICAgIG1f bGFzdFY4U3RyaW5nID0gaGFuZGxlOwogICAgICAgICAgICAgcmV0dXJuIHY4OjpMb2NhbDx2ODo6 U3RyaW5nPjo6TmV3KGhhbmRsZSk7CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9iaW5kaW5n cy92OC9WOFZhbHVlQ2FjaGUuaCBiL1NvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL3Y4L1Y4VmFsdWVD YWNoZS5oCmluZGV4IDgxNTRkZGY1MzIyMjczMjNlY2JhMDg4MDJlOWFkNGEzN2FhNmIzOWUuLmYy MTYwNjBjYzE0MDMzNWQ1MGUzYTcxNmJmYTczMTJlOTZkYzg4OWYgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJDb3JlL2JpbmRpbmdzL3Y4L1Y4VmFsdWVDYWNoZS5oCisrKyBiL1NvdXJjZS9XZWJDb3Jl L2JpbmRpbmdzL3Y4L1Y4VmFsdWVDYWNoZS5oCkBAIC00MywxMSArNDMsOCBAQCBwdWJsaWM6CiAK ICAgICB2ODo6TG9jYWw8djg6OlN0cmluZz4gdjhFeHRlcm5hbFN0cmluZyhTdHJpbmdJbXBsKiBz dHJpbmdJbXBsLCB2ODo6SXNvbGF0ZSogaXNvbGF0ZSkKICAgICB7Ci0gICAgICAgIGlmIChtX2xh c3RTdHJpbmdJbXBsLmdldCgpID09IHN0cmluZ0ltcGwpIHsKLSAgICAgICAgICAgIEFTU0VSVCgh bV9sYXN0VjhTdHJpbmcuSXNOZWFyRGVhdGgoKSk7Ci0gICAgICAgICAgICBBU1NFUlQoIW1fbGFz dFY4U3RyaW5nLklzRW1wdHkoKSk7CisgICAgICAgIGlmIChtX2xhc3RTdHJpbmdJbXBsLmdldCgp ID09IHN0cmluZ0ltcGwgJiYgbV9sYXN0VjhTdHJpbmcuSXNXZWFrKCkpCiAgICAgICAgICAgICBy ZXR1cm4gdjg6OkxvY2FsPHY4OjpTdHJpbmc+OjpOZXcobV9sYXN0VjhTdHJpbmcpOwotICAgICAg ICB9CiAKICAgICAgICAgcmV0dXJuIHY4RXh0ZXJuYWxTdHJpbmdTbG93KHN0cmluZ0ltcGwsIGlz b2xhdGUpOwogICAgIH0K