118533 2013-07-10 09:56:57 -0700 WebResourceCacheManager::cfURLCacheHostNamesWithCallback leaks an array (pointed out by the clang static analyzer) 2013-07-11 15:28:45 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 jberlin jberlin andersca jberlin jeffrey+webkit rniwa oldest_to_newest 907391 0 jberlin 2013-07-10 09:56:57 -0700 #if ENABLE(CACHE_PARTITIONING) void WebResourceCacheManager::cfURLCacheHostNamesWithCallback(CacheCallback callback) { WKCFURLCacheCopyAllPartitionNames(^(CFArrayRef partitionNames) { CFMutableArrayRef hostNames = CFArrayCreateMutableCopy(0, 0, WKCFURLCacheCopyAllHostNamesInPersistentStoreForPartition(CFSTR(""))); CFArrayAppendArray(hostNames, partitionNames, CFRangeMake(0, CFArrayGetCount(partitionNames))); CFRelease(partitionNames); callback(adoptCF(hostNames)); }); } #endif The array returned by the call to WKCFURLCacheCopyAllHostNamesInPersistentStoreForPartition(CFSTR("”)) is getting leaked. 907392 1 206395 jberlin 2013-07-10 10:03:25 -0700 Created attachment 206395 Patch 907443 2 206395 andersca 2013-07-10 13:02:55 -0700 Comment on attachment 206395 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=206395&action=review > Source/WebKit2/WebProcess/ResourceCache/cf/WebResourceCacheManagerCFNet.cpp:68 > + CFMutableArrayRef hostNames = CFArrayCreateMutableCopy(0, 0, hostNamesInPersistentStore.get()); I think you can use a RetainPtr here (with adoptCF). > Source/WebKit2/WebProcess/ResourceCache/cf/WebResourceCacheManagerCFNet.cpp:71 > callback(adoptCF(hostNames)); and then just call std::move(hostNames) here. 907680 3 jberlin 2013-07-11 09:21:51 -0700 (In reply to comment #2) > (From update of attachment 206395 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=206395&action=review > > > Source/WebKit2/WebProcess/ResourceCache/cf/WebResourceCacheManagerCFNet.cpp:68 > > + CFMutableArrayRef hostNames = CFArrayCreateMutableCopy(0, 0, hostNamesInPersistentStore.get()); > > I think you can use a RetainPtr here (with adoptCF). Done. > > > Source/WebKit2/WebProcess/ResourceCache/cf/WebResourceCacheManagerCFNet.cpp:71 > > callback(adoptCF(hostNames)); > > and then just call std::move(hostNames) here. Done. Thanks for the review! 907766 4 206395 jberlin 2013-07-11 15:28:34 -0700 Comment on attachment 206395 Patch Committed in http://trac.webkit.org/changeset/152578 206395 2013-07-10 10:03:25 -0700 2013-07-11 15:28:34 -0700 Patch patch.patch text/plain 1908 jberlin ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCAwZDc3MmQyLi5iMTkwMTNjIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTQg QEAKKzIwMTMtMDctMTAgIEplc3NpZSBCZXJsaW4gIDxqYmVybGluQGFwcGxlLmNvbT4KKworICAg ICAgICBXZWJSZXNvdXJjZUNhY2hlTWFuYWdlcjo6Y2ZVUkxDYWNoZUhvc3ROYW1lc1dpdGhDYWxs YmFjayBsZWFrcyBhbiBhcnJheSAocG9pbnRlZCBvdXQgYnkgdGhlCisgICAgICAgIGNsYW5nIHN0 YXRpYyBhbmFseXplcikKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTExODUzMworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgICogV2ViUHJvY2Vzcy9SZXNvdXJjZUNhY2hlL2NmL1dlYlJlc291cmNlQ2FjaGVNYW5h Z2VyQ0ZOZXQuY3BwOgorICAgICAgICAoV2ViS2l0OjpXZWJSZXNvdXJjZUNhY2hlTWFuYWdlcjo6 Y2ZVUkxDYWNoZUhvc3ROYW1lc1dpdGhDYWxsYmFjayk6CisKIDIwMTMtMDctMTAgIFNlcmdpbyBD b3JyZWlhICA8c2VyZ2lvLmNvcnJlaWFAb3BlbmJvc3NhLm9yZz4KIAogICAgICAgICBSZW1vdmUg dGhlIG1lc2hUeXBlIGZyb20gQ29vcmRpbmF0ZWRDdXN0b21GaWx0ZXJPcGVyYXRpb24KZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJLaXQyL1dlYlByb2Nlc3MvUmVzb3VyY2VDYWNoZS9jZi9XZWJSZXNv dXJjZUNhY2hlTWFuYWdlckNGTmV0LmNwcCBiL1NvdXJjZS9XZWJLaXQyL1dlYlByb2Nlc3MvUmVz b3VyY2VDYWNoZS9jZi9XZWJSZXNvdXJjZUNhY2hlTWFuYWdlckNGTmV0LmNwcAppbmRleCBkNjA1 ZmU3Li5jY2Q4M2I2IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL1Jlc291 cmNlQ2FjaGUvY2YvV2ViUmVzb3VyY2VDYWNoZU1hbmFnZXJDRk5ldC5jcHAKKysrIGIvU291cmNl L1dlYktpdDIvV2ViUHJvY2Vzcy9SZXNvdXJjZUNhY2hlL2NmL1dlYlJlc291cmNlQ2FjaGVNYW5h Z2VyQ0ZOZXQuY3BwCkBAIC02NCw3ICs2NCw4IEBAIFJldGFpblB0cjxDRkFycmF5UmVmPiBXZWJS ZXNvdXJjZUNhY2hlTWFuYWdlcjo6Y2ZVUkxDYWNoZUhvc3ROYW1lcygpCiB2b2lkIFdlYlJlc291 cmNlQ2FjaGVNYW5hZ2VyOjpjZlVSTENhY2hlSG9zdE5hbWVzV2l0aENhbGxiYWNrKENhY2hlQ2Fs bGJhY2sgY2FsbGJhY2spCiB7CiAgICAgV0tDRlVSTENhY2hlQ29weUFsbFBhcnRpdGlvbk5hbWVz KF4oQ0ZBcnJheVJlZiBwYXJ0aXRpb25OYW1lcykgewotICAgICAgICBDRk11dGFibGVBcnJheVJl ZiBob3N0TmFtZXMgPSBDRkFycmF5Q3JlYXRlTXV0YWJsZUNvcHkoMCwgMCwgV0tDRlVSTENhY2hl Q29weUFsbEhvc3ROYW1lc0luUGVyc2lzdGVudFN0b3JlRm9yUGFydGl0aW9uKENGU1RSKCIiKSkp OworICAgICAgICBSZXRhaW5QdHI8Q0ZBcnJheVJlZj4gaG9zdE5hbWVzSW5QZXJzaXN0ZW50U3Rv cmUgPSBhZG9wdENGKFdLQ0ZVUkxDYWNoZUNvcHlBbGxIb3N0TmFtZXNJblBlcnNpc3RlbnRTdG9y ZUZvclBhcnRpdGlvbihDRlNUUigiIikpKTsKKyAgICAgICAgQ0ZNdXRhYmxlQXJyYXlSZWYgaG9z dE5hbWVzID0gQ0ZBcnJheUNyZWF0ZU11dGFibGVDb3B5KDAsIDAsIGhvc3ROYW1lc0luUGVyc2lz dGVudFN0b3JlLmdldCgpKTsKICAgICAgICAgQ0ZBcnJheUFwcGVuZEFycmF5KGhvc3ROYW1lcywg cGFydGl0aW9uTmFtZXMsIENGUmFuZ2VNYWtlKDAsIENGQXJyYXlHZXRDb3VudChwYXJ0aXRpb25O YW1lcykpKTsKICAgICAgICAgQ0ZSZWxlYXNlKHBhcnRpdGlvbk5hbWVzKTsKICAgICAgICAgY2Fs bGJhY2soYWRvcHRDRihob3N0TmFtZXMpKTsK