Bug 134914

Summary: [Cocoa] _WKRemoteObjectInterface leaks NSString ivar
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: WebKit2Assignee: Joseph Pecoraro <joepeck>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, commit-queue, joepeck
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
[PATCH] Proposed Fix none

Joseph Pecoraro
Reported 2014-07-14 17:24:03 PDT
_WKRemoteObjectInterface copies _identifier in initialization, but doesn't have a -dealloc to release it. <rdar://problem/17672075>
Attachments
[PATCH] Proposed Fix (1.42 KB, patch)
2014-07-14 17:26 PDT, Joseph Pecoraro 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Joseph Pecoraro
Comment 1 2014-07-14 17:26:26 PDT
Created attachment 234895 [details] [PATCH] Proposed Fix Alternatively this could be an explicit ivar with RetainPtr.
Anders Carlsson
Comment 2 2014-07-14 17:27:28 PDT
Comment on attachment 234895 [details] [PATCH] Proposed Fix Please use a RetainPtr instead.
WebKit Commit Bot
Comment 3 2014-07-14 18:52:53 PDT
Comment on attachment 234895 [details] [PATCH] Proposed Fix Clearing flags on attachment: 234895 Committed r171098: <http://trac.webkit.org/changeset/171098>
WebKit Commit Bot
Comment 4 2014-07-14 18:52:56 PDT
All reviewed patches have been landed. Closing bug.
Joseph Pecoraro
Comment 5 2014-07-14 19:03:29 PDT
(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.
Joseph Pecoraro
Comment 6 2014-07-14 19:04:35 PDT
(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.
Joseph Pecoraro
Comment 7 2014-07-14 19:18:31 PDT
(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.
Anders Carlsson
Comment 8 2014-07-15 11:01:32 PDT
(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.
Note You need to log in before you can comment on or make changes to this bug.