1275117 - [e10s] leakcheck | default process: 8 bytes leaked (nsStringBuffer) when enabling NSPR logs

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Jean-Yves Avenard [:jya]]

Not sure if this is related to bug 1270752, nor if it's definitely IPC, but seeing that this issue only occurs with e10s on.

like here:
https://treeherder.mozilla.org/logviewer.html#?job_id=21278758&repo=try#L89077
https://treeherder.mozilla.org/logviewer.html#?job_id=21279204&repo=try
https://treeherder.mozilla.org/logviewer.html#?job_id=21279828&repo=try

STR:
enable NSPR logging by editing testing/mochitest/runtests.py and modify NSPR_LOG_MODULES
Start mochitest

All e10s run with debug build will show the leaks.

 07:52:05     INFO -  websocket/process bridge listening on port 8191
 07:52:05     INFO -  Stopping websocket/process bridge
 07:52:05     INFO -  TEST-INFO | leakcheck | default process: leak threshold set at 0 bytes
 07:52:05     INFO -  TEST-INFO | leakcheck | plugin process: leak threshold set at 0 bytes
 07:52:05     INFO -  TEST-INFO | leakcheck | tab process: leak threshold set at 10000 bytes
 07:52:05     INFO -  TEST-INFO | leakcheck | geckomediaplugin process: leak threshold set at 20000 bytes
 07:52:05     INFO -  == BloatView: ALL (cumulative) LEAK AND BLOAT STATISTICS, default process 1748
 07:52:05     INFO -       |<----------------Class--------------->|<-----Bytes------>|<----Objects---->|
 07:52:05     INFO -       |                                      | Per-Inst   Leaked|   Total      Rem|
 07:52:05     INFO -     0 |TOTAL                                 |       24        8| 4971024        1|
 07:52:05     INFO -  1215 |nsStringBuffer                        |        8        8|  177952        1|
 07:52:05     INFO -  nsTraceRefcnt::DumpStatistics: 1374 entries
 07:52:05     INFO -  TEST-INFO | leakcheck | default process: leaked 1 nsStringBuffer
 07:52:05  WARNING -  TEST-UNEXPECTED-FAIL | leakcheck | default process: 8 bytes leaked (nsStringBuffer)
 07:52:05     INFO -  == BloatView: ALL (cumulative) LEAK AND BLOAT STATISTICS, tab process 1750
 07:52:05     INFO -       |<----------------Class--------------->|<-----Bytes------>|<----Objects---->|
 07:52:05     INFO -       |                                      | Per-Inst   Leaked|   Total      Rem|
 07:52:05     INFO -     0 |TOTAL                                 |       22        0| 3768606        0|
 07:52:05     INFO -  nsTraceRefcnt::DumpStatistics: 943 entries

Comment 1

[Honza Bambas (:mayhemer)]

I think these are the static nsCString's I create at GeckoChildProcessHost::PerformAsyncLaunch.  Before that patch we were doing a silent strdup that has never been released and missed, because we don't track it that carefully as string buffers.

Need to figure out how to fix this...

Comment 2

[Honza Bambas (:mayhemer)]

I think converting the two static nsAutoCString restoreOrig*LogName to members of the class will do.  Even if GeckoChildProcessHost is not a singleton, we still revert the env to original state after the launch, so we are safe - we will read and cache the origin values again.

Comment 3

[Andrew McCreight (out of office until 8/21) [:mccr8]]

You can't use nsAutoCString in a class. There's a static analysis that will reject it. Maybe a regular nsCString will be okay?

Comment 4

[Honza Bambas (:mayhemer)]

(In reply to Andrew McCreight [:mccr8] from comment #3)
> You can't use nsAutoCString in a class. There's a static analysis that will
> reject it. Maybe a regular nsCString will be okay?

Of course.  Or direct buffers in UniquePtr's or something, whatever.

Comment 5

[Honza Bambas (:mayhemer)]

Attachment: v1

Jason, this is a super simple patch:

- moves the two strings we use to back the log file env vars from being static to being members of the class
- this makes them be released properly
- no issue if GeckoChildProcessHost is not a singleton, the values will always be the same and the method where they are used is not reentered 

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a1236a99ecdc

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5c028053889
Fix static strings leaks when mozlogging is on. r=jduell

Comment 7

[Carsten Book [:Tomcat]]

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