31637 2009-11-18 12:34:28 -0800 Need to ASSERT(isMainThread()) in ThreadShared methods. 2009-11-18 17:28:42 -0800 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 dimich dimich ap darin eric levin oldest_to_newest 164664 0 dimich 2009-11-18 12:34:28 -0800 Recent regression (bug 31615) showed that it's way too easy to pass references to Document and other TreeShared-derived classes using RefPtr/PassRefPtr between main thread and worker threads, causing refcounting of TreeShared to be used unsafe cross-thread. Need to add asserts in at least ref(0 and deref() methods of TreeShared since they are very easy to misuse. Consider adding asserts to other methods as mentioned in bug 31615. 164820 1 43471 dimich 2009-11-18 16:47:44 -0800 Created attachment 43471 Proposed patch. Added ASSERT to constructor, destructor, setParent, parent, ref and deref. Measured run-webkit-tests on debug build - the difference is not measurable. On my machine, it takes 11 min 20 sec, +/- 10 seconds from run to run, I can not see the difference. 164821 2 43471 darin 2009-11-18 16:48:28 -0800 Comment on attachment 43471 Proposed patch. r=me 164822 3 darin 2009-11-18 16:48:45 -0800 So this assertion is not hit at all anywhere in the regression tests? 164835 4 dimich 2009-11-18 17:22:06 -0800 (In reply to comment #3) > So this assertion is not hit at all anywhere in the regression tests? No, not now. It would before rollback of r50919. 164836 5 dimich 2009-11-18 17:28:42 -0800 Landed: http://trac.webkit.org/changeset/51158 43471 2009-11-18 16:47:44 -0800 2009-11-18 16:48:28 -0800 Proposed patch. patch.txt text/plain 2294 dimich ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg NmUwN2VjNC4uODM5NWJjMyAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxOSBAQAorMjAwOS0xMS0xOCAgRG1pdHJ5IFRpdG92 ICA8ZGltaWNoQGNocm9taXVtLm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9P UFMhKS4KKworICAgICAgICBOZWVkIHRvIEFTU0VSVChpc01haW5UaHJlYWQoKSkgaW4gVGhyZWFk U2hhcmVkIG1ldGhvZHMuCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVn LmNnaT9pZD0zMTYzNworCisgICAgICAgIEFkZGVkIEFTU0VSVChJc01haW5UaHJlYWQoKSkgdG8g YWxsIGZvbGxvd2luZyBtZXRob2RzOgorICAgICAgICAqIHBsYXRmb3JtL1RyZWVTaGFyZWQuaDoK KyAgICAgICAgKFdlYkNvcmU6OlRyZWVTaGFyZWQ6OlRyZWVTaGFyZWQpOgorICAgICAgICAoV2Vi Q29yZTo6VHJlZVNoYXJlZDo6flRyZWVTaGFyZWQpOgorICAgICAgICAoV2ViQ29yZTo6VHJlZVNo YXJlZDo6cmVmKToKKyAgICAgICAgKFdlYkNvcmU6OlRyZWVTaGFyZWQ6OmRlcmVmKToKKyAgICAg ICAgKFdlYkNvcmU6OlRyZWVTaGFyZWQ6OnNldFBhcmVudCk6CisgICAgICAgIChXZWJDb3JlOjpU cmVlU2hhcmVkOjpwYXJlbnQpOgorCiAyMDA5LTExLTE4ICBab2x0YW4gSG9ydmF0aCAgPHpvbHRh bkB3ZWJraXQub3JnPgogCiAgICAgICAgIFJldmlld2VkIGJ5IERhcmluIEFkbGVyLgpkaWZmIC0t Z2l0IGEvV2ViQ29yZS9wbGF0Zm9ybS9UcmVlU2hhcmVkLmggYi9XZWJDb3JlL3BsYXRmb3JtL1Ry ZWVTaGFyZWQuaAppbmRleCAwMjcyOGZmLi5hNjBhZDBkIDEwMDY0NAotLS0gYS9XZWJDb3JlL3Bs YXRmb3JtL1RyZWVTaGFyZWQuaAorKysgYi9XZWJDb3JlL3BsYXRmb3JtL1RyZWVTaGFyZWQuaApA QCAtMjMsNiArMjMsOSBAQAogCiAjaW5jbHVkZSA8d3RmL0Fzc2VydGlvbnMuaD4KICNpbmNsdWRl IDx3dGYvTm9uY29weWFibGUuaD4KKyNpZm5kZWYgTkRFQlVHCisjaW5jbHVkZSA8d3RmL1RocmVh ZGluZy5oPgorI2VuZGlmCiAKIG5hbWVzcGFjZSBXZWJDb3JlIHsKIApAQCAtMzIsNiArMzUsNyBA QCBwdWJsaWM6CiAgICAgICAgIDogbV9yZWZDb3VudChpbml0aWFsUmVmQ291bnQpCiAgICAgICAg ICwgbV9wYXJlbnQoMCkKICAgICB7CisgICAgICAgIEFTU0VSVChpc01haW5UaHJlYWQoKSk7CiAj aWZuZGVmIE5ERUJVRwogICAgICAgICBtX2RlbGV0aW9uSGFzQmVndW4gPSBmYWxzZTsKICAgICAg ICAgbV9pblJlbW92ZWRMYXN0UmVmRnVuY3Rpb24gPSBmYWxzZTsKQEAgLTM5LDExICs0MywxMyBA QCBwdWJsaWM6CiAgICAgfQogICAgIHZpcnR1YWwgflRyZWVTaGFyZWQoKQogICAgIHsKKyAgICAg ICAgQVNTRVJUKGlzTWFpblRocmVhZCgpKTsKICAgICAgICAgQVNTRVJUKG1fZGVsZXRpb25IYXNC ZWd1bik7CiAgICAgfQogCiAgICAgdm9pZCByZWYoKQogICAgIHsKKyAgICAgICAgQVNTRVJUKGlz TWFpblRocmVhZCgpKTsKICAgICAgICAgQVNTRVJUKCFtX2RlbGV0aW9uSGFzQmVndW4pOwogICAg ICAgICBBU1NFUlQoIW1faW5SZW1vdmVkTGFzdFJlZkZ1bmN0aW9uKTsKICAgICAgICAgKyttX3Jl ZkNvdW50OwpAQCAtNTEsNiArNTcsNyBAQCBwdWJsaWM6CiAKICAgICB2b2lkIGRlcmVmKCkKICAg ICB7CisgICAgICAgIEFTU0VSVChpc01haW5UaHJlYWQoKSk7CiAgICAgICAgIEFTU0VSVCghbV9k ZWxldGlvbkhhc0JlZ3VuKTsKICAgICAgICAgQVNTRVJUKCFtX2luUmVtb3ZlZExhc3RSZWZGdW5j dGlvbik7CiAgICAgICAgIGlmICgtLW1fcmVmQ291bnQgPD0gMCAmJiAhbV9wYXJlbnQpIHsKQEAg LTczLDggKzgwLDE3IEBAIHB1YmxpYzoKICAgICAgICAgcmV0dXJuIG1fcmVmQ291bnQ7CiAgICAg fQogCi0gICAgdm9pZCBzZXRQYXJlbnQoVCogcGFyZW50KSB7IG1fcGFyZW50ID0gcGFyZW50OyB9 Ci0gICAgVCogcGFyZW50KCkgY29uc3QgeyByZXR1cm4gbV9wYXJlbnQ7IH0KKyAgICB2b2lkIHNl dFBhcmVudChUKiBwYXJlbnQpCisgICAgeyAKKyAgICAgICAgQVNTRVJUKGlzTWFpblRocmVhZCgp KTsKKyAgICAgICAgbV9wYXJlbnQgPSBwYXJlbnQ7IAorICAgIH0KKworICAgIFQqIHBhcmVudCgp IGNvbnN0CisgICAgeworICAgICAgICBBU1NFUlQoaXNNYWluVGhyZWFkKCkpOworICAgICAgICBy ZXR1cm4gbV9wYXJlbnQ7CisgICAgfQogCiAjaWZuZGVmIE5ERUJVRwogICAgIGJvb2wgbV9kZWxl dGlvbkhhc0JlZ3VuOwo=