Created attachment 35001 [details] Make XHR leak more obvious While looking at the output of 'run-webkit-tests --leaks' over the last few days I've occasionally noticed instances of WebCore::XMLHttpRequest being leaked. I wrote a simple patch that uses RefCountedLeakCounter to make it more obvious when this sort of leak is occurring, and narrowed the leaks down to two tests that are run in sequence. The following command leaks a single XMLHttpRequest instance every time: ./WebKitTools/Scripts/run-webkit-tests --leaks LayoutTests/http/tests/xmlhttprequest/{xhr-onunload,xml-encoding}.html With the attached patch applied you'll see: LEAK: 1 XMLHttpRequest in the output.
I poked at this briefly and it seemed that the XHR instance gained an extra ref during a call to setPendingActivity which was never balanced out.
<rdar://problem/7149500>
This regressed in <http://trac.webkit.org/changeset/47291>, where a call to setPendingActivity() was moved to before ThreadableLoader::create(). Even worse, nonCacheRequestInFlight() call was lost in the refactoring.
Whoops, I will this.
Created attachment 35073 [details] Unfinished fix OK, here's what I already have - unfinished and untested. This also fixed a check before setReportUploadProgress() (which is only used by Chromium afaik).
Created attachment 35095 [details] Combination of previous two patches, plus ChangeLogs I verified this fixes the leak. Also, reran all LayoutTests, no regressions. And it looks correct, so not sure what else I can add.
Comment on attachment 35095 [details] Combination of previous two patches, plus ChangeLogs r=me
Landed in r47478.
Comment on attachment 35095 [details] Combination of previous two patches, plus ChangeLogs Rejecting patch 35095 from commit-queue. This patch will require manual commit. Patch https://bugs.webkit.org/attachment.cgi?id=35095 from bug 28412 failed to download and apply.
+ // For now we should only balance the nonCached request count for main-thread XHRs and not + // Worker XHRs, as the Cache is not thread-safe. + // This will become irrelevant after https://bugs.webkit.org/show_bug.cgi?id=27165 is resolved. By the way, it seems that moving this and related code to DocumentThreadableLoader could be a nice improvement while we wait for bug 27165.