Bug 136140 - Possible RetainPtr misuse in WKScriptMessage.mm - could leak
Summary: Possible RetainPtr misuse in WKScriptMessage.mm - could leak
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-08-21 17:41 PDT by Joseph Pecoraro
Modified: 2014-08-21 20:27 PDT (History)
4 users (show)

See Also:

Attachments
[PATCH] Proposed Fix (1.14 KB, patch)
2014-08-21 19:05 PDT, Joseph Pecoraro 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2014-08-21 17:41:26 PDT 
RetainPtr<...>::operator= will +1. This looks like a possible missing adoptNS which could cause a leak:

> Source/WebKit2/UIProcess/API/Cocoa/WKScriptMessage.mm:47:5:
>     _body = [body copy];
>     ^

However, since _body is RetainPtr<id>, how does this even work? "id" doesn't necessary respond to retain/release/copy. It looks like this could come from -[JSValue toObject] which returns id, but probably could return id<NSObject>.
Comment 1 Joseph Pecoraro 2014-08-21 18:39:15 PDT 
Maybe id is always guaranteed to at least be an NSObject. In that case we can just adoptNS.
Comment 2 Joseph Pecoraro 2014-08-21 19:05:55 PDT 
Created attachment 236957 [details]
[PATCH] Proposed Fix

This is an untested fix. Seeing as we are arbitrarily calling -copy, then -retain/-release should be fine for now.
Comment 3 WebKit Commit Bot 2014-08-21 20:27:15 PDT 
Comment on attachment 236957 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 236957

Committed r172846: <http://trac.webkit.org/changeset/172846>
Comment 4 WebKit Commit Bot 2014-08-21 20:27:18 PDT 
All reviewed patches have been landed.  Closing bug.