22380 2008-11-20 05:05:16 -0800 Fix WorkerContext refcounting 2008-11-20 12:37:08 -0800 1 1 1 Unclassified WebKit WebCore JavaScript 528+ (Nightly build) Mac OS X 10.5 RESOLVED FIXED P2 Normal --- 1 ap ap oldest_to_newest 99455 0 ap 2008-11-20 05:05:16 -0800 WorkerContext is not currently destroyed when its thread finishes, because its JS wrapper holds a reference to it. We need to clear destroy the JS heap before the thread finishes. 99456 1 25311 ap 2008-11-20 05:08:18 -0800 Created attachment 25311 proposed patch 99487 2 25311 darin 2008-11-20 09:24:59 -0800 Comment on attachment 25311 proposed patch > + , m_script(new WorkerScriptController(this)) For future consideration: We want to try to avoid bare "new" calls almost everywhere. For single-owner classes, I think a create function that returns auto_ptr is a good design. We can change OwnPtr so it has a constructor that takes an auto_ptr, if it doesn't already have one. For reference counted classes a create function that returns PassRefPtr is obviously our approach. > + workerContext->clearScript(); > + ASSERT(workerContext->refCount() == 1); I think you should add a comment here explaining why there's a guarantee that no one else is holding a reference. Our usual aim with reference-counted objects is to make the "shutdown" operation be something other than destruction, and always allow someone to prolong the lifetime of the object a bit, just to keep a pointer valid. I know we don't always use reference counting that way, but I think it merits a comment when we're not. By the way, can you use the hasOneRef() function here instead? r=me 99528 3 ap 2008-11-20 12:37:08 -0800 Committed revision 38626. Added a comment that hopefully clarifies the behavior a little, and changed to hasOneRef, which is better. 25311 2008-11-20 05:08:18 -0800 2008-11-20 09:24:59 -0800 proposed patch ClearWrapper.txt text/plain 3010 ap SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiAzODYxNykKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMjAgQEAKKzIwMDgtMTEtMjAgIEFsZXhleSBQcm9za3VyeWFrb3YgIDxhcEB3ZWJr aXQub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMjM4MAorICAgICAgICBG aXggV29ya2VyQ29udGV4dCByZWZjb3VudGluZworCisgICAgICAgICogZG9tL1dvcmtlckNvbnRl eHQuY3BwOgorICAgICAgICAoV2ViQ29yZTo6V29ya2VyQ29udGV4dDo6V29ya2VyQ29udGV4dCk6 CisgICAgICAgICogZG9tL1dvcmtlckNvbnRleHQuaDoKKyAgICAgICAgKFdlYkNvcmU6Oldvcmtl ckNvbnRleHQ6OnNjcmlwdCk6CisgICAgICAgIChXZWJDb3JlOjpXb3JrZXJDb250ZXh0OjpjbGVh clNjcmlwdCk6CisgICAgICAgIEFkZGVkIGEgbWV0aG9kIHRvIG1hbnVhbGx5IGRlc3Ryb3kgV29y a2VyU2N0cmlwdENvbnRyb2xsZXIuCisKKyAgICAgICAgKiBkb20vV29ya2VyVGhyZWFkLmNwcDog KFdlYkNvcmU6OldvcmtlclRocmVhZDo6d29ya2VyVGhyZWFkKToKKyAgICAgICAgRGVzdHJveSBX b3JrZXJTY3JpcHRDb250cm9sbGVyIHRvIHJlbGVhc2UgYW55IHJlZmVyZW5jZXMgdG8gV29ya2Vy Q29udGV4dC4KKwogMjAwOC0xMS0yMCAgQW50dGkgS29pdmlzdG8gIDxhbnR0aUBhcHBsZS5jb20+ CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgTWFjaWVqIFN0YWNob3dpYWsuCkluZGV4OiBXZWJDb3Jl L2RvbS9Xb3JrZXJDb250ZXh0LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL2RvbS9Xb3JrZXJD b250ZXh0LmNwcAkocmV2aXNpb24gMzg1OTUpCisrKyBXZWJDb3JlL2RvbS9Xb3JrZXJDb250ZXh0 LmNwcAkod29ya2luZyBjb3B5KQpAQCAtNDUsNyArNDUsNyBAQCBXb3JrZXJDb250ZXh0OjpXb3Jr ZXJDb250ZXh0KGNvbnN0IEtVUkwmCiAgICAgOiBtX3VybCh1cmwpCiAgICAgLCBtX2xvY2F0aW9u KFdvcmtlckxvY2F0aW9uOjpjcmVhdGUodXJsKSkKICAgICAsIG1fc2VjdXJpdHlPcmlnaW4oU2Vj dXJpdHlPcmlnaW46OmNyZWF0ZSh1cmwpKQotICAgICwgbV9zY3JpcHQodGhpcykKKyAgICAsIG1f c2NyaXB0KG5ldyBXb3JrZXJTY3JpcHRDb250cm9sbGVyKHRoaXMpKQogICAgICwgbV90aHJlYWQo dGhyZWFkKQogewogfQpJbmRleDogV2ViQ29yZS9kb20vV29ya2VyQ29udGV4dC5oCj09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT0KLS0tIFdlYkNvcmUvZG9tL1dvcmtlckNvbnRleHQuaAkocmV2aXNpb24gMzg1OTUpCisrKyBX ZWJDb3JlL2RvbS9Xb3JrZXJDb250ZXh0LmgJKHdvcmtpbmcgY29weSkKQEAgLTM1LDYgKzM1LDcg QEAKICNpbmNsdWRlICJLVVJMLmgiCiAjaW5jbHVkZSAiU2NyaXB0RXhlY3V0aW9uQ29udGV4dC5o IgogI2luY2x1ZGUgIldvcmtlclNjcmlwdENvbnRyb2xsZXIuaCIKKyNpbmNsdWRlIDx3dGYvT3du UHRyLmg+CiAjaW5jbHVkZSA8d3RmL1Bhc3NSZWZQdHIuaD4KICNpbmNsdWRlIDx3dGYvUmVmQ291 bnRlZC5oPgogI2luY2x1ZGUgPHd0Zi9SZWZQdHIuaD4KQEAgLTY0LDcgKzY1LDggQEAgbmFtZXNw YWNlIFdlYkNvcmUgewogCiAgICAgICAgIFdvcmtlckxvY2F0aW9uKiBsb2NhdGlvbigpIGNvbnN0 IHsgcmV0dXJuIG1fbG9jYXRpb24uZ2V0KCk7IH0KIAotICAgICAgICBXb3JrZXJTY3JpcHRDb250 cm9sbGVyKiBzY3JpcHQoKSB7IHJldHVybiAmbV9zY3JpcHQ7IH0KKyAgICAgICAgV29ya2VyU2Ny aXB0Q29udHJvbGxlciogc2NyaXB0KCkgeyByZXR1cm4gbV9zY3JpcHQuZ2V0KCk7IH0KKyAgICAg ICAgdm9pZCBjbGVhclNjcmlwdCgpIHsgcmV0dXJuIG1fc2NyaXB0LmNsZWFyKCk7IH0KICAgICAg ICAgV29ya2VyVGhyZWFkKiB0aHJlYWQoKSB7IHJldHVybiBtX3RocmVhZDsgfQogCiAgICAgICAg IGJvb2wgaGFzUGVuZGluZ0FjdGl2aXR5KCkgY29uc3Q7CkBAIC0xMDMsNyArMTA1LDcgQEAgbmFt ZXNwYWNlIFdlYkNvcmUgewogICAgICAgICBSZWZQdHI8V29ya2VyTG9jYXRpb24+IG1fbG9jYXRp b247CiAgICAgICAgIFJlZlB0cjxTZWN1cml0eU9yaWdpbj4gbV9zZWN1cml0eU9yaWdpbjsKIAot ICAgICAgICBXb3JrZXJTY3JpcHRDb250cm9sbGVyIG1fc2NyaXB0OworICAgICAgICBPd25QdHI8 V29ya2VyU2NyaXB0Q29udHJvbGxlcj4gbV9zY3JpcHQ7CiAgICAgICAgIFdvcmtlclRocmVhZCog bV90aHJlYWQ7CiAKICAgICAgICAgUmVmUHRyPEV2ZW50TGlzdGVuZXI+IG1fb25tZXNzYWdlTGlz dGVuZXI7CkluZGV4OiBXZWJDb3JlL2RvbS9Xb3JrZXJUaHJlYWQuY3BwCj09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0t IFdlYkNvcmUvZG9tL1dvcmtlclRocmVhZC5jcHAJKHJldmlzaW9uIDM4NTkwKQorKysgV2ViQ29y ZS9kb20vV29ya2VyVGhyZWFkLmNwcAkod29ya2luZyBjb3B5KQpAQCAtODgsNiArODgsOCBAQCB2 b2lkKiBXb3JrZXJUaHJlYWQ6OndvcmtlclRocmVhZCgpCiAgICAgICAgIHRhc2stPnBlcmZvcm1U YXNrKHdvcmtlckNvbnRleHQuZ2V0KCkpOwogICAgIH0KIAorICAgIHdvcmtlckNvbnRleHQtPmNs ZWFyU2NyaXB0KCk7CisgICAgQVNTRVJUKHdvcmtlckNvbnRleHQtPnJlZkNvdW50KCkgPT0gMSk7 CiAgICAgd29ya2VyQ29udGV4dCA9IDA7IC8vIERlc3Ryb3lpbmcgdGhlIGNvbnRleHQgd2lsbCBu b3RpZnkgbWVzc2FnaW5nIHByb3h5LgogICAgIG1fdGhyZWFkSUQgPSAwOwogCg==