1344584 - test_be_conservative.js leaks TLSServerSocket and the world around it

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[(no longer active)]

Attachment: WIP patch

$ XPCOM_MEM_LEAK_LOG=1 ./mach xpcshell-test netwerk/test/unit/test_be_conservative.js

 0:04.18 PROCESS_OUTPUT: Thread-1 (pid:66839) "== BloatView: ALL (cumulative) LEAK STATISTICS, default process 66839"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) "     |<----------------Class--------------->|<-----Bytes------>|<----Objects---->|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) "     |                                      | Per-Inst   Leaked|   Total      Rem|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) "   0 |TOTAL                                 |       54     8088|   22583      108|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) "   8 |BackstagePass                         |      104      104|      12        1|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) "  30 |CancelableRunnable                    |       48       96|      44        2|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) "  38 |CondVar                               |       40       80|      70        2|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) "  64 |IdlePeriod                            |       24       24|      28        1|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) "  79 |Mutex                                 |       32      288|     249        9|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) "  86 |PollableEvent                         |       24       24|       1        1|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) "  99 |ReentrantMonitor                      |       40      160|      41        4|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) " 106 |Runnable                              |       40       80|     351        2|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) " 121 |TLSServerConnectionInfo               |      128      256|       4        2|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) " 122 |TLSServerSecurityObserverProxy        |       32       64|       4        2|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) " 123 |TLSServerSocket                       |      240      240|       2        1|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) " 148 |XPCNativeInterface                    |       56      504|      65        9|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) " 149 |XPCNativeMember                       |       16      144|    1873        9|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) " 150 |XPCNativeSet                          |       32      160|      83        5|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) " 151 |XPCWrappedNative                      |       96      480|     252        5|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) " 152 |XPCWrappedNativeProto                 |       40      200|      90        5|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) " 154 |XPCWrappedNativeTearOff               |       32      160|     305        5|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) " 161 |nsAStreamCopier                       |      176      352|       8        2|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) " 236 |nsJSPrincipals                        |       24       24|      20        1|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) " 243 |nsMainThreadPtrHolder<T>              |       24       48|      24        2|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) " 247 |nsNSSCertificate                      |       72       72|       7        1|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) " 262 |nsPipe                                |      208      832|      11        4|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) " 263 |nsPipeInputStream                     |      136      544|      11        4|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) " 278 |nsServerSocket                        |      224      224|       2        1|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) " 283 |nsSocketTransport                     |      760     1520|       9        2|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) " 284 |nsSocketTransportService              |      256      256|       1        1|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) " 290 |nsStringBuffer                        |        8       24|    8134        3|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) " 296 |nsTArray_base                         |        8       64|    4098        8|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) " 297 |nsThread                              |      352      352|      27        1|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) " 323 |nsXPCWrappedJS                        |      120      240|      15        2|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) " 324 |nsXPCWrappedJSClass                   |       72       72|      12        1|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) " 329 |xptiInterfaceInfo                     |       40      400|      73       10|"
 0:04.19 PROCESS_OUTPUT: Thread-1 (pid:66839) "nsTraceRefcnt::DumpStatistics: 330 entries"

I spent a few hours today trying to debug this using refcount tracing logs.  As far as I can tell, the main object being leaked is TLSServerSocket.  My first theory was that there is a JS reference to this object which is leaking, but I kept eliminating them one by one, and I got to the patch attached but still the leak happens.  Note that the test creates two server objects, and only the second one leaks.  At this point I'm pretty sure the leak is coming from something inside Necko, but I don't have any good theories.  Unfortunately the refcount tracing log for TLSServerSocket is too complicated for me to understand.

I'm stuck on this one, would appreciate some help please.

Comment 1

[Patrick McManus [:mcmanus]]

I think this was yours david? remind me if it was mine :)

Comment 2

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Yes, I wrote this test. Looks like I wasn't closing the streams in the right places in the error case. I'll have a patch shortly.

Comment 3

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Attachment: bug 1344584 - proactively close streams in test_be_conservative.js to prevent leaks

Review commit: https://reviewboard.mozilla.org/r/122460/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/122460/

Comment 4

[Patrick McManus [:mcmanus]]

Comment on attachment 8849683 [details]
bug 1344584 - proactively close streams in test_be_conservative.js to prevent leaks

https://reviewboard.mozilla.org/r/122460/#review124886

thanks!

Comment 5

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Thanks!
Here's try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=071849e7e71be334b7181191695d5aafeeda843d

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d624f51d6ff3
proactively close streams in test_be_conservative.js to prevent leaks r=mcmanus

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d624f51d6ff3