459620 - can leak a nsHttpHandler when authentication fails

Status: RESOLVED DUPLICATE of bug 475053

(Core :: Networking: HTTP, defect)

Description

[Justin Dolske [:Dolske]]

Found after checking in bug 394611 caused a leak on Windows and OS X (but not Linux, oddly enough). Trace-malloc reports:

leaked 4215 bytes during test execution (should have leaked no more than 0 bytes)
leaked 3 instances of StringAdopt with size 1 bytes each (3 bytes total)
leaked 1 instance of ThreadShutdownObserver with size 16 bytes
leaked 2 instances of mozStorageConnection with size 76 bytes each (152 bytes total)
leaked 1 instance of mozStorageService with size 28 bytes
leaked 6 instances of mozStorageStatement with size 44 bytes each (264 bytes total)
leaked 2 instances of nsCategoryObserver with size 60 bytes each (120 bytes total)
leaked 1 instance of nsCookiePermission with size 32 bytes
leaked 1 instance of nsCookieService with size 100 bytes
leaked 1 instance of nsDNSRecord with size 28 bytes
leaked 1 instance of nsDNSService with size 44 bytes
leaked 1 instance of nsEffectiveTLDService with size 48 bytes
leaked 1 instance of nsHashtable with size 44 bytes
leaked 1 instance of nsHostRecord with size 52 bytes
leaked 1 instance of nsHttpConnection with size 88 bytes
leaked 1 instance of nsHttpConnectionInfo with size 40 bytes 
leaked 1 instance of nsHttpHandler with size 344 bytes
leaked 1 instance of nsIDNService with size 64 bytes
leaked 1 instance of nsIOService with size 124 bytes
leaked 2 instances of nsLocalFile with size 28 bytes each (56 bytes total)
leaked 1 instance of nsNetworkLinkService with size 28 bytes 
leaked 1 instance of nsObserverService with size 48 bytes 
leaked 1 instance of nsPermissionManager with size 92 bytes
leaked 1 instance of nsPrefBranch with size 56 bytes
leaked 1 instance of nsSocketTransport with size 292 bytes
leaked 1 instance of nsSocketTransportService with size 1676 bytes
leaked 22 instances of nsStringBuffer with size 8 bytes each (176 bytes total)
leaked 1 instance of nsTArray_base with size 4 bytes
leaked 1 instance of nsThreadPool with size 60 bytes
leaked 1 instance of nsUnicodeNormalizer with size 12 bytes
leaked 11 instances of nsVoidArray with size 4 bytes each (44 bytes total) 
leaked 5 instances of nsWeakReference with size 16 bytes each (80 bytes total)

I started with looking at the mozStorageService (since that's the only pwmgr related thing). It's held onto by nsCookieService, which in turn is held onto by a nsHttpConnection (which looks likely to be the root leak, I think).

Running the pwmgr test_prompt.html mochitest (as modified in bug 394611) reliably reproduces the leak. The specific sequence of actions causing the leak seems to be:

1) Authenticate to example.com, realm "realm1"
2) Authenticate to example.com, realm "realm2"
3) Authenticate to example.com, realm "realm2" (but with the site now expecting a different password, so first request fails and we prompt the user for new credentials)

Steps 1/2/3 correspond to subtests 1003, 1004, and 1005 in test_prompt.html. If I skip the last one (1005), the test finishes leak free. Also interesting to note that steps 2 and 3 alone are not enough to cause the leak -- jumping directly to subtest 1004 from at beginning doesn't leak, but jumping to 1003 does.

Comment 1

[Justin Dolske [:Dolske]]

Attachment: HTTP log from test_prompt.html run

Comment 2

[Justin Dolske [:Dolske]]

Forgot to add: if I call nsIHttpAuthManager.clearAll() after step 2, there's no leak.

Comment 3

[Honza Bambas (:mayhemer)]

This may be fixed (worked around) by patch for bug 475053.

Comment 4

[Honza Bambas (:mayhemer)]

To describe the leak in detail: when we got 403 from the server (we didn't send any or any correct credentials to authenticate and it is required) we try to keep the connection for re-authentication attempt. But, when the connection was closed by the server (it is not Connection: keep-alive type) we still try to re-use that connection but the nsHttpConnectionManager throws that connection away and give us (the transaction) silently a new one. The bug lies in the nsHttpConnectionManager that forgets (on a place I haven't found yet) Release() on the nsHttpConnection in this particular case.

A fix (or better say a workaround) is to check the IsPersistent flag of the connection before we try to re-use it in the nsHttpChannel. See bug 475053 for the patch.