134914 2014-07-14 17:24:03 -0700 [Cocoa] _WKRemoteObjectInterface leaks NSString ivar 2014-07-15 11:01:32 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 joepeck joepeck andersca commit-queue joepeck oldest_to_newest 1022370 0 joepeck 2014-07-14 17:24:03 -0700 _WKRemoteObjectInterface copies _identifier in initialization, but doesn't have a -dealloc to release it. <rdar://problem/17672075> 1022371 1 234895 joepeck 2014-07-14 17:26:26 -0700 Created attachment 234895 [PATCH] Proposed Fix Alternatively this could be an explicit ivar with RetainPtr. 1022372 2 234895 andersca 2014-07-14 17:27:28 -0700 Comment on attachment 234895 [PATCH] Proposed Fix Please use a RetainPtr instead. 1022389 3 234895 commit-queue 2014-07-14 18:52:53 -0700 Comment on attachment 234895 [PATCH] Proposed Fix Clearing flags on attachment: 234895 Committed r171098: <http://trac.webkit.org/changeset/171098> 1022390 4 commit-queue 2014-07-14 18:52:56 -0700 All reviewed patches have been landed. Closing bug. 1022392 5 joepeck 2014-07-14 19:03:29 -0700 (In reply to comment #2) > (From update of attachment 234895 [details]) > Please use a RetainPtr instead. Heh, you should have cq-. I'll follow-up. 1022393 6 joepeck 2014-07-14 19:04:35 -0700 (In reply to comment #5) > (In reply to comment #2) > > (From update of attachment 234895 [details] [details]) > > Please use a RetainPtr instead. > > Heh, you should have cq-. I'll follow-up. Oh, I think there was a review collision and the flags got overwritten. 1022398 7 joepeck 2014-07-14 19:18:31 -0700 (In reply to comment #5) > (In reply to comment #2) > > (From update of attachment 234895 [details] [details]) > > Please use a RetainPtr instead. > > Heh, you should have cq-. I'll follow-up. The identifier property is (as written) atomic, so making an explicit RetainPtr<NSString> ivar and reimplementing the getter atomically seems like unnecessary extra work considering the simplicity of the current patch. Let me know if you feel strongly about this, or perhaps the atomic is unnecessary. 1022512 8 andersca 2014-07-15 11:01:32 -0700 (In reply to comment #7) > (In reply to comment #5) > > (In reply to comment #2) > > > (From update of attachment 234895 [details] [details] [details]) > > > Please use a RetainPtr instead. > > > > Heh, you should have cq-. I'll follow-up. > > The identifier property is (as written) atomic, so making an explicit RetainPtr<NSString> ivar and reimplementing the getter atomically seems like unnecessary extra work considering the simplicity of the current patch. Let me know if you feel strongly about this, or perhaps the atomic is unnecessary. Make it non-atomic instead. 234895 2014-07-14 17:26:26 -0700 2014-07-14 18:52:53 -0700 [PATCH] Proposed Fix proposed-fix.patch text/plain 1457 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCBjMTMzYzE5Li4yNGRlNzJhIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUg QEAKKzIwMTQtMDctMTQgIEpvc2VwaCBQZWNvcmFybyAgPHBlY29yYXJvQGFwcGxlLmNvbT4KKwor ICAgICAgICBbQ29jb2FdIF9XS1JlbW90ZU9iamVjdEludGVyZmFjZSBsZWFrcyBOU1N0cmluZyBp dmFyCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMzQ5 MTQKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBSZWxl YXNlIG91ciBjb3BpZWQgTlNTdHJpbmcgaW4gZGVhbGxvYy4KKworICAgICAgICAqIFNoYXJlZC9B UEkvQ29jb2EvX1dLUmVtb3RlT2JqZWN0SW50ZXJmYWNlLm1tOgorICAgICAgICAoLVtfV0tSZW1v dGVPYmplY3RJbnRlcmZhY2UgZGVhbGxvY10pOgorCiAyMDE0LTA3LTE0ICBUaW0gSG9ydG9uICA8 dGltb3RoeV9ob3J0b25AYXBwbGUuY29tPgogCiAgICAgICAgIEFTU0VSVChpc01haW5UaHJlYWQo KSkgdW5kZXIgT25lU2hvdERpc3BsYXlMaW5rSGFuZGxlcgpkaWZmIC0tZ2l0IGEvU291cmNlL1dl YktpdDIvU2hhcmVkL0FQSS9Db2NvYS9fV0tSZW1vdGVPYmplY3RJbnRlcmZhY2UubW0gYi9Tb3Vy Y2UvV2ViS2l0Mi9TaGFyZWQvQVBJL0NvY29hL19XS1JlbW90ZU9iamVjdEludGVyZmFjZS5tbQpp bmRleCBjNGQ0MTk1Li42NTAzMTcyIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9TaGFyZWQv QVBJL0NvY29hL19XS1JlbW90ZU9iamVjdEludGVyZmFjZS5tbQorKysgYi9Tb3VyY2UvV2ViS2l0 Mi9TaGFyZWQvQVBJL0NvY29hL19XS1JlbW90ZU9iamVjdEludGVyZmFjZS5tbQpAQCAtMTQxLDYg KzE0MSwxMiBAQCBzdGF0aWMgdm9pZCBpbml0aWFsaXplQWxsb3dlZEFyZ3VtZW50Q2xhc3Nlcyhf V0tSZW1vdGVPYmplY3RJbnRlcmZhY2UgKmludGVyZmFjZQogICAgIHJldHVybiBbW1tzZWxmIGFs bG9jXSBpbml0V2l0aFByb3RvY29sOnByb3RvY29sIGlkZW50aWZpZXI6TlNTdHJpbmdGcm9tUHJv dG9jb2wocHJvdG9jb2wpXSBhdXRvcmVsZWFzZV07CiB9CiAKKy0gKHZvaWQpZGVhbGxvYworewor ICAgIFtfaWRlbnRpZmllciByZWxlYXNlXTsKKyAgICBbc3VwZXIgZGVhbGxvY107Cit9CisKIC0g KE5TU3RyaW5nICopZGVzY3JpcHRpb24KIHsKICAgICByZXR1cm4gW05TU3RyaW5nIHN0cmluZ1dp dGhGb3JtYXQ6QCI8JUA6ICVwOyBwcm90b2NvbCA9IFwiJUBcIjsgaWRlbnRpZmllciA9IFwiJUBc Ij4iLCBOU1N0cmluZ0Zyb21DbGFzcyhzZWxmLmNsYXNzKSwgc2VsZiwgX2lkZW50aWZpZXIsIE5T U3RyaW5nRnJvbVByb3RvY29sKF9wcm90b2NvbCldOwo=