Closed Bug 1404482 Opened 7 years ago Closed 6 years ago

Small permaleak on Windows in content processes (nsStringBuffer, nsTArray_base)

Categories

(Core :: IPC: MSCOM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox-esr52 --- unaffected
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- wontfix
firefox60 --- fixed

People

(Reporter: mccr8, Assigned: bugzilla)

References

(Blocks 1 open bug)

Details

(Keywords: memory-leak, Whiteboard: [MemShrink:P2])

Attachments

(1 file)

Windows seems to have a permaleak across basically all tests:

== BloatView: ALL (cumulative) LEAK AND BLOAT STATISTICS, tab process 3860
     |<----------------Class--------------->|<-----Bytes------>|<----Objects---->|
     |                                      | Per-Inst   Leaked|   Total      Rem|
   0 |TOTAL                                 |       34       16|  384532        2|
 702 |nsStringBuffer                        |        8        8|   35622        1|
 743 |nsTArray_base                         |        8        8|   97457        1|

This is important mostly because it prevents us from reducing the leak threshold to zero for content processes.
This is a 16 bytes leak, so I could reduce the leak threshold to 16 to avoid worrying about it.
This is probably old, but it is hard to say.

The best way to investigate this is probably to track the set of live nsStringBuffers, and then print out the live ones at shutdown, or maybe to just use the existing mechanism to print out the allocation stack (though maybe that's broken?). The content of the string will likely indicate what is wrong. There's probably a small bug with some global variable that holds a string.
Whiteboard: [MemShrink]
Whiteboard: [MemShrink] → [MemShrink:P2]
Can you provide steps for testing locally on Windows?
Flags: needinfo?(continuation)
(In reply to Eric Rahm [:erahm] (please no mozreview requests) from comment #3)
> Can you provide steps for testing locally on Windows?

I think running the browser with
  XPCOM_MEM_LEAK_LOG=1 XPCOM_MEM_LOG_CLASSES=nsStringBuffer,nsTArray_base
will give you allocation stacks.

I can also do this on try, but if you have a Windows dev set up locally that would be easier. Thanks.
Flags: needinfo?(continuation)
Serial Numbers of Leaked Objects:
5 @0000027A25A5C080 (1 references)
allocation stack:
#00: nsStringBuffer::Alloc (c:\dev\mozilla-unified\xpcom\string\nssubstring.cpp:269)
#01: nsTSubstring<char>::MutatePrep (c:\dev\mozilla-unified\xpcom\string\nstsubstring.cpp:165)
#02: nsTSubstring<char>::ReplacePrepInternal (c:\dev\mozilla-unified\xpcom\string\nstsubstring.cpp:228)
#03: nsTSubstring<char>::ReplacePrep (c:\dev\mozilla-unified\xpcom\string\nstsubstring.cpp:219)
#04: nsTSubstring<char>::Assign (c:\dev\mozilla-unified\xpcom\string\nstsubstring.cpp:389)
#05: nsTSubstring<char>::Assign (c:\dev\mozilla-unified\xpcom\string\nstsubstring.cpp:485)
#06: nsTSubstring<char>::Assign (c:\dev\mozilla-unified\xpcom\string\nstsubstring.cpp:440)
#07: CrashReporter::DelayedNote::DelayedNote (c:\dev\mozilla-unified\toolkit\crashreporter\nsexceptionhandler.cpp:2208)
#08: CrashReporter::AnnotateCrashReport (c:\dev\mozilla-unified\toolkit\crashreporter\nsexceptionhandler.cpp:2269)
#09: mozilla::mscom::MainThreadRuntime::MainThreadRuntime (c:\dev\mozilla-unified\ipc\mscom\mainthreadruntime.cpp:67)
#10: XRE_InitChildProcess (c:\dev\mozilla-unified\toolkit\xre\nsembedfunctions.cpp:649)
#11: content_process_main (c:\dev\mozilla-unified\ipc\contentproc\plugin-container.cpp:64)
#12: NS_internal_main (c:\dev\mozilla-unified\browser\app\nsbrowserapp.cpp:283)
#13: wmain (c:\dev\mozilla-unified\toolkit\xre\nswindowswmain.cpp:114)
#14: __scrt_common_main_seh (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:283)
#15: BaseThreadInitThunk[C:\Windows\System32\KERNEL32.DLL +0x8364]
#16: RtlUserThreadStart[C:\Windows\SYSTEM32\ntdll.dll +0x67091]
TEST-INFO | leakcheck | tab process: leaked 1 nsStringBuffer
TEST-INFO | leakcheck | tab process: leaked 1 nsTArray_base
leakcheck | tab process: 20 bytes leaked (nsStringBuffer, nsTArray_base)

== BloatView: ALL (cumulative) LEAK AND BLOAT STATISTICS, tab process 8772

     |<----------------Class--------------->|<-----Bytes------>|<----Objects---->|
     |                                      | Per-Inst   Leaked|   Total      Rem|
   0 |TOTAL                                 |       51       20|   61919        2|
 677 |nsStringBuffer                        |       12       12|   17969        1|
 716 |nsTArray_base                         |        8        8|   13923        1|

nsTraceRefcnt::DumpStatistics: 779 entries

Serial Numbers of Leaked Objects:
5 @0000026BBE45C080 (1 references)
allocation stack:
#00: nsStringBuffer::Alloc (c:\dev\mozilla-unified\xpcom\string\nssubstring.cpp:269)
#01: nsTSubstring<char>::MutatePrep (c:\dev\mozilla-unified\xpcom\string\nstsubstring.cpp:165)
#02: nsTSubstring<char>::ReplacePrepInternal (c:\dev\mozilla-unified\xpcom\string\nstsubstring.cpp:228)
#03: nsTSubstring<char>::ReplacePrep (c:\dev\mozilla-unified\xpcom\string\nstsubstring.cpp:219)
#04: nsTSubstring<char>::Assign (c:\dev\mozilla-unified\xpcom\string\nstsubstring.cpp:389)
#05: nsTSubstring<char>::Assign (c:\dev\mozilla-unified\xpcom\string\nstsubstring.cpp:485)
#06: nsTSubstring<char>::Assign (c:\dev\mozilla-unified\xpcom\string\nstsubstring.cpp:440)
#07: CrashReporter::DelayedNote::DelayedNote (c:\dev\mozilla-unified\toolkit\crashreporter\nsexceptionhandler.cpp:2208)
#08: CrashReporter::AnnotateCrashReport (c:\dev\mozilla-unified\toolkit\crashreporter\nsexceptionhandler.cpp:2269)
#09: mozilla::mscom::MainThreadRuntime::MainThreadRuntime (c:\dev\mozilla-unified\ipc\mscom\mainthreadruntime.cpp:67)
#10: XRE_InitChildProcess (c:\dev\mozilla-unified\toolkit\xre\nsembedfunctions.cpp:649)
#11: content_process_main (c:\dev\mozilla-unified\ipc\contentproc\plugin-container.cpp:64)
#12: NS_internal_main (c:\dev\mozilla-unified\browser\app\nsbrowserapp.cpp:283)
#13: wmain (c:\dev\mozilla-unified\toolkit\xre\nswindowswmain.cpp:114)
#14: __scrt_common_main_seh (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:283)
#15: BaseThreadInitThunk[C:\Windows\System32\KERNEL32.DLL +0x8364]
#16: RtlUserThreadStart[C:\Windows\SYSTEM32\ntdll.dll +0x67091]
TEST-INFO | leakcheck | tab process: leaked 1 nsStringBuffer
TEST-INFO | leakcheck | tab process: leaked 1 nsTArray_base
leakcheck | tab process: 20 bytes leaked (nsStringBuffer, nsTArray_base)
Looks like gDelayedAnnotations is destroyed during startup, but not again, so if any more delayed notes get added, you'd get a leak. I don't know why this is Windows-only.
Component: XPCOM → Crash Reporting
Product: Core → Toolkit
The annotation is from the MSCOM stuff, which is why it is only on Windows:
    CrashReporter::AnnotateCrashReport(NS_LITERAL_CSTRING("AssemblyManifestCtx"),
                                       strActCtx);

But I don't know why we don't leak otherwise. Maybe this gets called at a weird point where we don't clean it up correctly.
(In reply to Andrew McCreight [:mccr8] from comment #9)
> The annotation is from the MSCOM stuff, which is why it is only on Windows:
>    
> CrashReporter::AnnotateCrashReport(NS_LITERAL_CSTRING("AssemblyManifestCtx"),
>                                        strActCtx);
> 
> But I don't know why we don't leak otherwise. Maybe this gets called at a
> weird point where we don't clean it up correctly.

Aaron, it looks like you know this code [1]. Can you take a look? I've confirmed commenting out those lines removes the leak, but it's not clear to me what the best option for cleaning up the leaked array is (I don't see any sort of cleanup mechanism that is actually called after startup).

[1] http://searchfox.org/mozilla-central/rev/423b2522c48e1d654e30ffc337164d677f934ec3/ipc/mscom/MainThreadRuntime.cpp#65
Flags: needinfo?(aklotz)
Sure. I think part of the problem is that this code might be executing before the crash reporter is fully initialized.
Assignee: nobody → aklotz
Status: NEW → ASSIGNED
Flags: needinfo?(aklotz)
Component: Crash Reporting → IPC: MSCOM
Product: Toolkit → Core
I'm just going to remove this annotation for now. If we really need it, we can resurrect it using a different mechanism.
Comment on attachment 8944819 [details]
Bug 1404482: Remove crash report annotation that was being made before the crash reporter initialized;

https://reviewboard.mozilla.org/r/214974/#review220724
Attachment #8944819 - Flags: review?(jteh) → review+
Pushed by aklotz@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ed9e9171bf0f
Remove crash report annotation that was being made before the crash reporter initialized; r=Jamie
https://hg.mozilla.org/mozilla-central/rev/ed9e9171bf0f
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Doesn't seem worth backporting to Beta, but feel free to nominate it if you feel strongly otherwise.
Nah, I don't think we need to uplift.
Flags: needinfo?(aklotz)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: