Bug 65789 - Leak in CFNetwork Loader RetainPtr<> should Adopt a Copy allocation
Summary: Leak in CFNetwork Loader RetainPtr<> should Adopt a Copy allocation
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-05 13:57 PDT by Joseph Pecoraro
Modified: 2011-08-05 17:25 PDT (History)
5 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (1.73 KB, patch)
2011-08-05 14:00 PDT, Joseph Pecoraro
darin: review+
joepeck: commit-queue-
Details | Formatted Diff | Diff
[PATCH] Fix Build Issue With Previous Patch (1.76 KB, patch)
2011-08-05 14:48 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2011-08-05 13:57:10 PDT
Both of these:

    if (RetainPtr<CFDataRef> bodyData = CFURLRequestCopyHTTPRequestBody(request))
         return FormData::create(CFDataGetBytePtr(bodyData.get()), CFDataGetLength(bodyData.get()));
 
    if (RetainPtr<CFArrayRef> bodyParts = wkCFURLRequestCopyHTTPRequestBodyParts(request)) {
         RefPtr<FormData> formData = FormData::create();

Cause leaks because RetainPtr::operation= will CFRetain() but FooCopyBar() already returns
a +1 allocation. We should adopt the reference with AdoptCF.
Comment 1 Joseph Pecoraro 2011-08-05 14:00:18 PDT
Created attachment 103104 [details]
[PATCH] Proposed Fix
Comment 2 Joseph Pecoraro 2011-08-05 14:30:07 PDT
Comment on attachment 103104 [details]
[PATCH] Proposed Fix

cq- based on the bot failing to build on Windows.
Comment 3 Joseph Pecoraro 2011-08-05 14:34:09 PDT
Windows build bot says:
http://queues.webkit.org/results/9323095

3>####### COMPILING 2 FILES USING AT MOST 8 PARALLEL INSTANCES OF cl.exe ###########
3>FormDataStreamCFNet.cpp
3>..\platform\network\cf\FormDataStreamCFNet.cpp(94) : error C2061: syntax error : identifier 'AdoptCF'
3>..\platform\network\cf\FormDataStreamCFNet.cpp(94) : error C2059: syntax error : ')'
3>..\platform\network\cf\FormDataStreamCFNet.cpp(95) : error C2143: syntax error : missing ';' before 'return'
3>..\platform\network\cf\FormDataStreamCFNet.cpp(95) : error C2065: 'bodyData' : undeclared identifier
3>..\platform\network\cf\FormDataStreamCFNet.cpp(95) : error C2228: left of '.get' must have class/struct/union
3>        type is ''unknown-type''
3>..\platform\network\cf\FormDataStreamCFNet.cpp(95) : error C2228: left of '.get' must have class/struct/union
3>        type is ''unknown-type''

This built fine for me locally. Maybe the Window's compiler doesn't like the shorthand
constructor syntax inside of an if statement? I could move the assignments outside
of the if statements if needed. Adding some windows developers.
Comment 4 Joseph Pecoraro 2011-08-05 14:34:33 PDT
s/if statement/if condition/
Comment 5 Joseph Pecoraro 2011-08-05 14:48:44 PDT
Created attachment 103117 [details]
[PATCH] Fix Build Issue With Previous Patch

I forgot that by default PLATFORM(MAC) doesn't take this path by default.
So my local build was a false positive. I've now tested building this change
correctly.
Comment 6 David Kilzer (:ddkilzer) 2011-08-05 16:24:46 PDT
Comment on attachment 103117 [details]
[PATCH] Fix Build Issue With Previous Patch

r=me
Comment 7 WebKit Review Bot 2011-08-05 17:25:05 PDT
Comment on attachment 103117 [details]
[PATCH] Fix Build Issue With Previous Patch

Clearing flags on attachment: 103117

Committed r92530: <http://trac.webkit.org/changeset/92530>
Comment 8 WebKit Review Bot 2011-08-05 17:25:09 PDT
All reviewed patches have been landed.  Closing bug.