Bug 65806

Summary: Potential Leaks - RetainPtr<> over retaining Create'd objects
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: WebKit Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, joepeck, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
[PATCH] Potential Fix (Untested, just code inspection and change) none

Description Joseph Pecoraro 2011-08-05 19:35:19 PDT
shell> cd Source/
shell> ack 'Retain.*?=.*?Create'

    WebCore/platform/network/mac/NetworkStateNotifierMac.cpp
    108:    RetainPtr<CFRunLoopSourceRef> configSource = SCDynamicStoreCreateRunLoopSource(0, m_store.get(), 0);
    
    WebCore/plugins/mac/PluginPackageMac.cpp
    123:        WTF::RetainPtr<CFStringRef> str = CFStringCreateWithPascalString(0, p, encoding);
    143:        WTF::RetainPtr<CFStringRef> path = CFStringCreateWithFormat(0, 0, CFSTR("%@/Library/Preferences/%@"), homeDir.get(), fileName.get());
    
    WebKit/mac/WebView/WebView.mm
    4215:    RetainPtr<CFMutableSetRef> visitedViews = CFSetCreateMutable(0, 0, 0);

The first looks harmless, the plugin ones look like it could be bad in encountered (in a loop),
and the WebView one looks straightforward if ever encountered.
Comment 1 Joseph Pecoraro 2011-08-05 19:46:56 PDT
Created attachment 103141 [details]
[PATCH] Potential Fix (Untested, just code inspection and change)

Each of these looks like they should be an Adopt after reading
the immediately surrounding code.
Comment 2 Darin Adler 2011-08-06 07:52:18 PDT
Did you do the same grep for Copy?
Comment 3 WebKit Review Bot 2011-08-06 09:17:28 PDT
Comment on attachment 103141 [details]
[PATCH] Potential Fix (Untested, just code inspection and change)

Clearing flags on attachment: 103141

Committed r92552: <http://trac.webkit.org/changeset/92552>
Comment 4 WebKit Review Bot 2011-08-06 09:17:32 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Joseph Pecoraro 2011-08-07 11:32:08 PDT
(In reply to comment #2)
> Did you do the same grep for Copy?

Yep that turned up both of these:
<http://webkit.org/b/65789> Leak in CFNetwork Loader RetainPtr<> should Adopt a Copy allocation
<http://webkit.org/b/65790> [QT] Possible Leaks WKRetainPtr<> should Adopt allocated Copy

Thanks!