72089 2011-11-10 21:50:59 -0800 JSString forgets to clear m_fibers when resolving ropes 2011-11-10 22:40:15 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 fpizlo webkit-unassigned ggaren webkit.review.bot oldest_to_newest 500515 0 fpizlo 2011-11-10 21:50:59 -0800 This can cause some pathological memory usage. Patch on the way. 500516 1 114626 fpizlo 2011-11-10 21:52:07 -0800 Created attachment 114626 the patch 500518 2 webkit.review.bot 2011-11-10 22:01:41 -0800 Attachment 114626 did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 Source/JavaScriptCore/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style. 500520 3 ggaren 2011-11-10 22:07:00 -0800 Might be good to merge your new comment with the comment above, which (now) incorrectly ASSERTs that using a Vector is OK because the string will mark its fibers -- really, using a Vector is only OK because GC just won't happen. r=me 500521 4 114626 ggaren 2011-11-10 22:08:03 -0800 Comment on attachment 114626 the patch View in context: https://bugs.webkit.org/attachment.cgi?id=114626&action=review > Source/JavaScriptCore/runtime/JSString.cpp:109 > + // Clearing here works only because there are no GC points in this method. Might be good to merge this with the comment above, which ASSERTs that using a Vector is OK -- really, using a Vector is only OK because GC just won't happen. 500524 5 fpizlo 2011-11-10 22:11:06 -0800 (In reply to comment #4) > (From update of attachment 114626 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=114626&action=review > > > Source/JavaScriptCore/runtime/JSString.cpp:109 > > + // Clearing here works only because there are no GC points in this method. > > Might be good to merge this with the comment above, which ASSERTs that using a Vector is OK -- really, using a Vector is only OK because GC just won't happen. Oh, heh, didn't even notice that comment. I've gone for two comments, one to say that it's OK to put them into the Vector (because there are no GC points) and another to say that it's OK to clear m_fibers (because there are no GC points). Figure that minimizes the chances of someone getting the wrong ideas. 500533 6 fpizlo 2011-11-10 22:40:15 -0800 Landed in http://trac.webkit.org/changeset/99927 114626 2011-11-10 21:52:07 -0800 2011-11-10 22:08:03 -0800 the patch fixstring_patch_1.diff text/plain 1488 fpizlo SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gOTk5MjUpCisrKyBTb3VyY2Uv SmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTIgQEAK KzIwMTEtMTEtMTAgIEZpbGlwIFBpemxvICA8ZnBpemxvQGFwcGxlLmNvbT4KKworICAgICAgICBO ZWVkIGEgc2hvcnQgZGVzY3JpcHRpb24gYW5kIGJ1ZyBVUkwgKE9PUFMhKQorCisgICAgICAgIFJl dmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogcnVudGltZS9KU1N0cmluZy5j cHA6CisgICAgICAgIChKU0M6OkpTU3RyaW5nOjpyZXNvbHZlUm9wZVNsb3dDYXNlKToKKwogMjAx MS0xMS0wOSAgRmlsaXAgUGl6bG8gIDxmcGl6bG9AYXBwbGUuY29tPgogCiAgICAgICAgIERGRyBi eXRlIGFycmF5IHN1cHBvcnQgc29tZXRpbWVzIGNsYW1wcyB2YWx1ZXMgaW5jb3JyZWN0bHkKSW5k ZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0pTU3RyaW5nLmNwcAo9PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 Ci0tLSBTb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9KU1N0cmluZy5jcHAJKHJldmlzaW9u IDk5OTI1KQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvSlNTdHJpbmcuY3BwCSh3 b3JraW5nIGNvcHkpCkBAIC0xMDQsOCArMTA0LDExIEBAIHZvaWQgSlNTdHJpbmc6OnJlc29sdmVS b3BlU2xvd0Nhc2UoRXhlY1MKICAgICBVQ2hhciogcG9zaXRpb24gPSBidWZmZXIgKyBtX2xlbmd0 aDsgLy8gV2Ugd2lsbCBiZSB3b3JraW5nIGJhY2t3YXJkcyBvdmVyIHRoZSByb3BlLgogICAgIFZl Y3RvcjxKU1N0cmluZyosIDMyPiB3b3JrUXVldWU7IC8vIFRoZXNlIHN0cmluZ3MgYXJlIGtlcHQg YWxpdmUgYnkgdGhlIHBhcmVudCByb3BlLCBzbyB1c2luZyBhIFZlY3RvciBpcyBPSy4KICAgICAK LSAgICBmb3IgKHNpemVfdCBpID0gMDsgaSA8IHNfbWF4SW50ZXJuYWxSb3BlTGVuZ3RoICYmIG1f ZmliZXJzW2ldOyArK2kpCisgICAgZm9yIChzaXplX3QgaSA9IDA7IGkgPCBzX21heEludGVybmFs Um9wZUxlbmd0aCAmJiBtX2ZpYmVyc1tpXTsgKytpKSB7CiAgICAgICAgIHdvcmtRdWV1ZS5hcHBl bmQobV9maWJlcnNbaV0uZ2V0KCkpOworICAgICAgICAvLyBDbGVhcmluZyBoZXJlIHdvcmtzIG9u bHkgYmVjYXVzZSB0aGVyZSBhcmUgbm8gR0MgcG9pbnRzIGluIHRoaXMgbWV0aG9kLgorICAgICAg ICBtX2ZpYmVyc1tpXS5jbGVhcigpOworICAgIH0KIAogICAgIHdoaWxlICghd29ya1F1ZXVlLmlz RW1wdHkoKSkgewogICAgICAgICBKU1N0cmluZyogY3VycmVudEZpYmVyID0gd29ya1F1ZXVlLmxh c3QoKTsK