Closed Bug 773255 Opened 12 years ago Closed 12 years ago

Intermittent mochitest-browser-chrome leak of 5023, 8163, or 8171 bytes (4 AtomImpl, 1 MemoryReporter_StorageSQLite, 8 Mutex, 1 QuotaCallback, 4 QuotaCallbackData etc)

Categories

(Core :: Networking: HTTP, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: emorley, Assigned: mcmanus)

References

Details

(Keywords: intermittent-failure, memory-leak, Whiteboard: [MemShrink:P3])

Attachments

(1 file)

Rev3 MacOSX Leopard 10.5.8 mozilla-central debug test mochitest-other on 2012-07-11 10:24:42 PDT for push e4857e5dfb51 slave: talos-r3-leopard-026 https://tbpl.mozilla.org/php/getParsedLog.php?id=13432826&tree=Firefox { == BloatView: ALL (cumulative) LEAK AND BLOAT STATISTICS, default process 329 |<----------------Class--------------->|<-----Bytes------>|<----------------Objects---------------->|<--------------References-------------->| Per-Inst Leaked Total Rem Mean StdDev Total Rem Mean StdDev 0 TOTAL 17 5023 478024100 179 (16465.50 +/- 26797.19) 576660141 100 (15994482867.11 +/- 0.00) 13 AtomImpl 24 96 60853 4 ( 3422.38 +/- 939.45) 4900432 12 (30923.29 +/- 8859.71) 184 MemoryReporter_StorageSQLite 12 12 1 1 ( 1.00 +/- 0.00) 11 1 ( 3.67 +/- 1.74) 191 Mutex 12 96 29919 8 ( 292.69 +/- 83.97) 0 0 ( 0.00 +/- 0.00) 222 QuotaCallback 12 12 1 1 ( 1.00 +/- 0.00) 18 4 ( 3.81 +/- 1.28) 223 QuotaCallbackData 8 32 8 4 ( 2.67 +/- 0.89) 0 0 ( 0.00 +/- 0.00) 232 ReentrantMonitor 16 32 15999 2 ( 227.37 +/- 85.88) 0 0 ( 0.00 +/- 0.00) 247 Service 68 68 1 1 ( 1.00 +/- 0.00) 1702 1 ( 22.77 +/- 2.89) 254 SpdySession3 456 456 3 1 ( 1.00 +/- 0.71) 6 2 ( 1.90 +/- 1.20) 261 StorageSQLiteMultiReporter 52 52 1 1 ( 1.00 +/- 0.00) 7 1 ( 2.38 +/- 1.04) 262 StringAdopt 1 3 356508 3 ( 5.44 +/- 8.62) 0 0 ( 0.00 +/- 0.00) 273 ThirdPartyUtil 16 16 1 1 ( 1.00 +/- 0.00) 48 1 ( 2.91 +/- 0.79) 276 TransportSecurityInfo 140 140 756 1 ( 55.88 +/- 39.48) 19113 1 ( 159.37 +/- 125.25) 459 nsCStringKey 20 600 1093393 30 ( 2270.02 +/- 823.23) 0 0 ( 0.00 +/- 0.00) 472 nsCategoryObserver 64 128 52 2 ( 13.32 +/- 8.81) 1318 2 ( 49.18 +/- 36.42) 519 nsCookiePermission 36 36 1 1 ( 1.00 +/- 0.00) 38 1 ( 7.28 +/- 2.86) 521 nsCookieService 80 80 1 1 ( 1.00 +/- 0.00) 1241 1 ( 8.11 +/- 3.29) 527 nsDNSRecord 32 32 2041 1 ( 2.08 +/- 2.15) 8745 1 ( 3.21 +/- 2.47) 528 nsDNSService 84 84 1 1 ( 1.00 +/- 0.00) 3215 1 ( 23.85 +/- 1.74) 596 nsDeque 52 104 656208 2 ( 75.02 +/- 42.93) 0 0 ( 0.00 +/- 0.00) 666 nsEffectiveTLDService 48 48 1 1 ( 1.00 +/- 0.00) 9876 2 ( 8.23 +/- 2.29) 806 nsHashKey 8 240 3651352 30 ( 4350.45 +/- 1383.98) 0 0 ( 0.00 +/- 0.00) 808 nsHashtable 44 44 10202 1 ( 1060.84 +/- 371.10) 0 0 ( 0.00 +/- 0.00) 810 nsHostRecord 68 68 42 1 ( 8.51 +/- 6.84) 4358 1 ( 15.10 +/- 8.47) 839 nsHttpConnection 176 176 1560 1 ( 2.26 +/- 1.98) 35815 1 ( 9.93 +/- 8.84) 840 nsHttpConnectionInfo 40 40 2605 1 ( 58.89 +/- 39.63) 10700 1 ( 63.46 +/- 41.24) 841 nsHttpConnectionMgr 188 188 1 1 ( 1.00 +/- 0.00) 8523 1 ( 2.34 +/- 0.78) 844 nsHttpHandler 420 420 1 1 ( 1.00 +/- 0.00) 157361 1 ( 54.83 +/- 51.59) 847 nsIDNService 64 64 1 1 ( 1.00 +/- 0.00) 3092 4 ( 7.04 +/- 0.90) 850 nsIOService 136 136 1 1 ( 1.00 +/- 0.00) 916351 1 ( 66.68 +/- 36.41) 943 nsNSSCertificate 60 60 1780 1 ( 118.23 +/- 94.45) 13226 1 ( 303.82 +/- 305.20) 946 nsNSSSocketInfo 188 188 448 1 ( 48.11 +/- 35.02) 14907 1 ( 159.73 +/- 123.47) 964 nsNetworkLinkService 28 28 1 1 ( 1.00 +/- 0.00) 7 1 ( 2.85 +/- 1.21) 978 nsObserverService 48 48 1 1 ( 1.00 +/- 0.00) 334385 4 ( 66.34 +/- 34.69) 999 nsPermissionManager 96 96 1 1 ( 1.00 +/- 0.00) 4731 2 ( 8.72 +/- 3.52) 1014 nsPrefBranch 76 76 1130 1 ( 28.74 +/- 15.03) 11225 1 ( 42.96 +/- 24.03) 1058 nsSSLStatus 52 52 731 1 ( 55.87 +/- 38.54) 7690 1 ( 153.67 +/- 124.74) 1169 nsSocketTransport 312 312 1564 1 ( 2.42 +/- 2.38) 41188 3 ( 11.94 +/- 10.20) 1170 nsSocketTransportService 120 120 1 1 ( 1.00 +/- 0.00) 51180 2 ( 13.19 +/- 2.71) 1180 nsStreamConverterService 16 16 1 1 ( 1.00 +/- 0.00) 130 1 ( 2.95 +/- 0.75) 1185 nsStrictTransportSecurityService 60 60 1 1 ( 1.00 +/- 0.00) 697 1 ( 3.99 +/- 0.72) 1186 nsStringBuffer 8 288 7026625 36 (39249.61 +/- 9103.86) 19894616 36 (60922.84 +/- 13671.01) 1235 nsTArray_base 4 16 78645686 4 (73002.70 +/- 18606.02) 0 0 ( 0.00 +/- 0.00) 1280 nsUnicodeNormalizer 12 12 1 1 ( 1.00 +/- 0.00) 4 1 ( 1.71 +/- 0.76) 1296 nsVoidArray 4 68 3186889 17 (13938.05 +/- 4921.38) 0 0 ( 0.00 +/- 0.00) 1299 nsWeakReference 16 80 78854 5 ( 1832.42 +/- 730.22) 1512026 5 ( 6229.91 +/- 1934.24) }
Summary: Intermittent mochitest-browser-chrome of 5023 bytes (4 AtomImpl, 1 MemoryReporter_StorageSQLite, 8 Mutex, 1 QuotaCallback, 4 QuotaCallbackData etc) → Intermittent mochitest-browser-chrome leak of 5023 bytes (4 AtomImpl, 1 MemoryReporter_StorageSQLite, 8 Mutex, 1 QuotaCallback, 4 QuotaCallbackData etc)
Ed, is this a new leak?
Whiteboard: [orange] → [orange][MemShrink]
Whiteboard: [orange][MemShrink] → [orange][MemShrink:P3]
Summary: Intermittent mochitest-browser-chrome leak of 5023 bytes (4 AtomImpl, 1 MemoryReporter_StorageSQLite, 8 Mutex, 1 QuotaCallback, 4 QuotaCallbackData etc) → Intermittent mochitest-browser-chrome leak of 5023 or 8163 bytes (4 AtomImpl, 1 MemoryReporter_StorageSQLite, 8 Mutex, 1 QuotaCallback, 4 QuotaCallbackData etc)
Summary: Intermittent mochitest-browser-chrome leak of 5023 or 8163 bytes (4 AtomImpl, 1 MemoryReporter_StorageSQLite, 8 Mutex, 1 QuotaCallback, 4 QuotaCallbackData etc) → Intermittent mochitest-browser-chrome leak of 5023, 8163, or 8171 bytes (4 AtomImpl, 1 MemoryReporter_StorageSQLite, 8 Mutex, 1 QuotaCallback, 4 QuotaCallbackData etc)
This has regressed significantly recently. First of the new occurrences on OS X 10.8 is: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&jobname=Rev5%20MacOSX%20Mountain%20Lion%2010.8%20mozilla-inbound%20debug%20test%20mochitest-other&rev=70ced7d14f3c Have retriggered a loads of runs prior.
(In reply to Ed Morley (Away 18th-20th) [:edmorley UTC+1] from comment #27) > This has regressed significantly recently. > > First of the new occurrences on OS X 10.8 is: > https://tbpl.mozilla.org/?tree=Mozilla- > Inbound&jobname=Rev5%20MacOSX%20Mountain%20Lion%2010.8%20mozilla- > inbound%20debug%20test%20mochitest-other&rev=70ced7d14f3c > > Have retriggered a loads of runs prior. Was caused by this push: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&jobname=Rev5%20MacOSX%20Mountain%20Lion%2010.8%20mozilla-inbound%20debug%20test%20mochitest-other&rev=6d71ff5b4b36 Have backed out 3 of the 4 csets; hopefully should do the trick :-)
Depends on: 792511
this bug blocks 769764 because checking in that code turns this from random orange to often orange on os x 10.8. AFAICT that's coincidental due to a change in timing. the key item being leaked here is the nsHttpConnection.. I'm not sure if the path is the same as 792511 or not. in the orange iteration of the test a spdy session is established to www.google.com and that connection fails a IsAlive() check at some point. I don't really know why that happens, but it happens in the green version too so tracking it down is a low roi activity [I suspect it needs security callbacks that have gone away]. This forces the spdysession to close, and the connection to get passed back to the connection manger via the reclaim logic. It can't be reused (IsAlive() fails after all) so connection->close() is called and then the entry is removed from the connection manager list along with that reference. Normally that's the last ref and the connection object dtors and takes the spdy object and the transport object and the reference to the protocol handler with it. And when the test is green, that's what happens. But when its orange there is still a ref on the nshttpconnection object. It turns out to be the nsIAsyncOutputStream (aka mSocketOut) has a callback to nsHttpConnection (as it also implements nsIOutputStreamCallback) and nothing clears that up. So we sit there. The obvious fix (clearing the callbacks in mSocketIn and mSocketOut during errors in close()) results in a different leak; so a fix isn't done yet.
Blocks: 769764
Component: Storage → Networking: HTTP
Product: Toolkit → Core
Attached patch patch 0Splinter Review
(Christian, this one is kind of time sensitive because it blocks other work - and its just 2 lines long. if you could look at it ASAP I would appreciate it.) clear the callbacks in mSocketOut on the error path of nsHttpConnection::Close(). mSocketIn is not treated similarly because the closed socket on the input path will result in a successful EOF callback that clears mSocketIn naturally.
Assignee: nobody → mcmanus
Status: NEW → ASSIGNED
Attachment #665385 - Flags: review?(cbiesinger)
try runs of interest: https://tbpl.mozilla.org/?tree=Try&rev=86ddfa4181ff shows just this patch for regression testing https://tbpl.mozilla.org/?tree=Try&rev=1d426c2ccd70 shows this patch plus 769764 for a large number of mochi-other on osx 10.8 debug without generating the leak which is common with 769764 without this patch on that platform https://tbpl.mozilla.org/?tree=Try&rev=7d9eb7dbad66 shows this patch with the other reviewed proxy patches (the complete push) cross platform for regression testing purposes.
Comment on attachment 665385 [details] [diff] [review] patch 0 Review of attachment 665385 [details] [diff] [review]: ----------------------------------------------------------------- Disclaimer: I don't see a problem with this, but I also cannot confirm there isn't any ;) ::: netwerk/protocol/http/nsHttpConnection.cpp @@ +488,5 @@ > mSocketTransport->SetSecurityCallbacks(nullptr); > mSocketTransport->SetEventSink(nullptr, nullptr); > mSocketTransport->Close(reason); > + if (mSocketOut) > + mSocketOut->AsyncWait(nullptr, 0, 0, nullptr); Could it happen that this releases the last reference? If there is some suspicion, then I would rather set mKeepAlive = false before all these unassignments.
Attachment #665385 - Flags: review?(cbiesinger) → review+
Honza, thanks for looking at this! (In reply to Honza Bambas (:mayhemer) from comment #36) >; > > Could it happen that this releases the last reference? > > If there is some suspicion, then I would rather set mKeepAlive = false > before all these unassignments. I think that can't happen - the caller should have a reference to the object. if that weren't the case we'd already be crashing on the mKeepalive reference, or even the mSocketTransport->Close() deref of this->mSocketTransport as SetEventSink(nullptr) would have dropped the last ref to the connection object.. (because generally mSocket[In,Out] are not holding a reference, though this bug shows there are exceptions)
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Whiteboard: [orange][MemShrink:P3] → [MemShrink:P3]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: