155515 2016-03-15 15:14:38 -0700 [Win] Correct double-release of CFURLConnectionRef 2016-03-15 17:00:10 -0700 1 1 1 Unclassified WebKit WebKit Misc. WebKit Nightly Build PC All RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=155522 InRadar P2 Normal --- 1 bfulgham bfulgham bfulgham webkit-bug-importer oldest_to_newest 1175136 0 bfulgham 2016-03-15 15:14:38 -0700 A double-release of a CFURLConnectionRef was identified in one of the WebDownload::init methods: CFURLConnectionRef connection = handle->connection(); … m_download = adoptCF(CFURLDownloadCreateAndStartWithLoadingConnection(0, connection, request.cfURLRequest(UpdateHTTPBody), response.cfURLResponse(), &client)); … // The CFURLDownload either starts successfully and retains the CFURLConnection, // or it fails to creating and we have a now-useless connection with a dangling ref. // Either way, we need to release the connection to balance out ref counts handle->releaseConnectionForDownload(); CFRelease(connection); The last line, the call to CFRelease(connection), is wrong and should be removed, because ResourceHandle::connection() just does d->m_connection.get() CFURLDownloadCreateAndStartWithLoadingConnection() can retain the connection per the comment, while ResourceHandle:: releaseConnectionForDownload() lets go of the connection. But then we release the connection via the raw pointer we stole from the ResourceHandle, as if we thought ResourceHandle::connection() returned a retained connection! 1175137 1 bfulgham 2016-03-15 15:15:30 -0700 <rdar://problem/25159143> 1175143 2 bfulgham 2016-03-15 15:19:33 -0700 Note: It looks like this code is not tested in the LayoutTest system because the 'http/tests/downloads/' test suite is skipped on Windows due to missing DRT features. 1175144 3 274141 bfulgham 2016-03-15 15:21:34 -0700 Created attachment 274141 Patch 1175149 4 274143 bfulgham 2016-03-15 15:25:24 -0700 Created attachment 274143 Patch 1175175 5 274150 bfulgham 2016-03-15 16:05:02 -0700 Created attachment 274150 Patch 1175200 6 bfulgham 2016-03-15 16:59:50 -0700 Testing infrastructure on Windows is needed to avoid breaking this in the future. See Bug 155522. 1175201 7 bfulgham 2016-03-15 17:00:10 -0700 Committed r198244: <http://trac.webkit.org/changeset/198244> 274141 2016-03-15 15:21:34 -0700 2016-03-15 15:24:53 -0700 Patch bug-155515-20160315154439.patch text/plain 217 bfulgham SW5kZXg6IFNvdXJjZS9XZWJLaXQvd2luL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09CkluZGV4OiBTb3Vy Y2UvV2ViS2l0L3dpbi9XZWJEb3dubG9hZENGTmV0LmNwcAo9PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Cg== 274143 2016-03-15 15:25:24 -0700 2016-03-15 16:04:59 -0700 Patch double_release.patch text/plain 1116 bfulgham SW5kZXg6IENoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBDaGFuZ2VMb2cJKHJldmlzaW9uIDE5ODIz MSkKKysrIENoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE1IEBACisyMDE2LTAz LTE1ICBCcmVudCBGdWxnaGFtICA8YmZ1bGdoYW1AYXBwbGUuY29tPgorCisgICAgICAgIFtXaW5d IENvcnJlY3QgZG91YmxlLXJlbGVhc2Ugb2YgQ0ZVUkxDb25uZWN0aW9uUmVmCisgICAgICAgIGh0 dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNTU1MTUKKyAgICAgICAgPHJk YXI6Ly9wcm9ibGVtLzI1MTU5MTQzPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09Q UyEpLgorCisgICAgICAgIFRlc3RlZCBieSBodHRwL3Rlc3RzL2Rvd25sb2FkIHN1aXRlLgorCisg ICAgICAgICogV2ViRG93bmxvYWRDRk5ldC5jcHA6IFJlbW92ZSBleHRyYSBDRlJlbGVhc2UuCisK IDIwMTYtMDMtMTQgIFBlciBBcm5lIFZvbGxhbiAgPHBlYXZvQG91dGxvb2suY29tPgogCiAgICAg ICAgIFtXaW5DYWlyb10gQ29tcGlsZSBmaXguCkluZGV4OiBXZWJEb3dubG9hZENGTmV0LmNwcAo9 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09Ci0tLSBXZWJEb3dubG9hZENGTmV0LmNwcAkocmV2aXNpb24gMTk4MjMwKQorKysg V2ViRG93bmxvYWRDRk5ldC5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTk2LDcgKzk2LDYgQEAKICAg ICAvLyBvciBpdCBmYWlscyB0byBjcmVhdGluZyBhbmQgd2UgaGF2ZSBhIG5vdy11c2VsZXNzIGNv bm5lY3Rpb24gd2l0aCBhIGRhbmdsaW5nIHJlZi4gCiAgICAgLy8gRWl0aGVyIHdheSwgd2UgbmVl ZCB0byByZWxlYXNlIHRoZSBjb25uZWN0aW9uIHRvIGJhbGFuY2Ugb3V0IHJlZiBjb3VudHMKICAg ICBoYW5kbGUtPnJlbGVhc2VDb25uZWN0aW9uRm9yRG93bmxvYWQoKTsKLSAgICBDRlJlbGVhc2Uo Y29ubmVjdGlvbik7CiB9CiAKIHZvaWQgV2ViRG93bmxvYWQ6OmluaXQoY29uc3QgVVJMJiB1cmws IElXZWJEb3dubG9hZERlbGVnYXRlKiBkZWxlZ2F0ZSkK 274150 2016-03-15 16:05:02 -0700 2016-03-15 16:59:04 -0700 Patch bug-155515-20160315160451.patch text/plain 1265 bfulgham SW5kZXg6IFNvdXJjZS9XZWJLaXQvd2luL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2Uv V2ViS2l0L3dpbi9DaGFuZ2VMb2cJKHJldmlzaW9uIDE5ODIxNikKKysrIFNvdXJjZS9XZWJLaXQv d2luL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE1IEBACisyMDE2LTAzLTE1 ICBCcmVudCBGdWxnaGFtICA8YmZ1bGdoYW1AYXBwbGUuY29tPgorCisgICAgICAgIFtXaW5dIENv cnJlY3QgZG91YmxlLXJlbGVhc2Ugb2YgQ0ZVUkxDb25uZWN0aW9uUmVmCisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNTU1MTUKKyAgICAgICAgPHJkYXI6 Ly9wcm9ibGVtLzI1MTU5MTQzPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEp LgorCisgICAgICAgIFRlc3RlZCBieSBodHRwL3Rlc3RzL2Rvd25sb2FkIHN1aXRlLgorCisgICAg ICAgICogV2ViRG93bmxvYWRDRk5ldC5jcHA6IFJlbW92ZSBleHRyYSBDRlJlbGVhc2UuCisKIDIw MTYtMDMtMTQgIFBlciBBcm5lIFZvbGxhbiAgPHBlYXZvQG91dGxvb2suY29tPgogCiAgICAgICAg IFtXaW5DYWlyb10gQ29tcGlsZSBmaXguCkluZGV4OiBTb3VyY2UvV2ViS2l0L3dpbi9XZWJEb3du bG9hZENGTmV0LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViS2l0L3dpbi9XZWJEb3dubG9h ZENGTmV0LmNwcAkocmV2aXNpb24gMTk4MjE2KQorKysgU291cmNlL1dlYktpdC93aW4vV2ViRG93 bmxvYWRDRk5ldC5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTk2LDcgKzk2LDYgQEAgdm9pZCBXZWJE b3dubG9hZDo6aW5pdChSZXNvdXJjZUhhbmRsZSogaAogICAgIC8vIG9yIGl0IGZhaWxzIHRvIGNy ZWF0aW5nIGFuZCB3ZSBoYXZlIGEgbm93LXVzZWxlc3MgY29ubmVjdGlvbiB3aXRoIGEgZGFuZ2xp bmcgcmVmLiAKICAgICAvLyBFaXRoZXIgd2F5LCB3ZSBuZWVkIHRvIHJlbGVhc2UgdGhlIGNvbm5l Y3Rpb24gdG8gYmFsYW5jZSBvdXQgcmVmIGNvdW50cwogICAgIGhhbmRsZS0+cmVsZWFzZUNvbm5l Y3Rpb25Gb3JEb3dubG9hZCgpOwotICAgIENGUmVsZWFzZShjb25uZWN0aW9uKTsKIH0KIAogdm9p ZCBXZWJEb3dubG9hZDo6aW5pdChjb25zdCBVUkwmIHVybCwgSVdlYkRvd25sb2FkRGVsZWdhdGUq IGRlbGVnYXRlKQo=