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)
Core
IPC: MSCOM
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.
Reporter | ||
Comment 1•7 years ago
|
||
This is a 16 bytes leak, so I could reduce the leak threshold to 16 to avoid worrying about it.
Updated•7 years ago
|
status-firefox55:
--- → unaffected
status-firefox56:
--- → unaffected
status-firefox57:
--- → affected
status-firefox58:
--- → affected
status-firefox-esr52:
--- → unaffected
Keywords: mlk
Reporter | ||
Comment 2•7 years ago
|
||
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.
status-firefox55:
unaffected → ---
status-firefox56:
unaffected → ---
status-firefox-esr52:
unaffected → ---
Reporter | ||
Updated•7 years ago
|
Whiteboard: [MemShrink]
Updated•7 years ago
|
Whiteboard: [MemShrink] → [MemShrink:P2]
Comment 3•7 years ago
|
||
Can you provide steps for testing locally on Windows?
Flags: needinfo?(continuation)
Reporter | ||
Comment 4•7 years ago
|
||
(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)
Comment hidden (obsolete) |
Comment hidden (obsolete) |
Comment 7•7 years ago
|
||
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)
Reporter | ||
Comment 8•7 years ago
|
||
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
Reporter | ||
Comment 9•7 years ago
|
||
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.
Comment 10•7 years ago
|
||
(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
Updated•7 years ago
|
Flags: needinfo?(aklotz)
Assignee | ||
Comment 11•7 years ago
|
||
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)
Comment 12•6 years ago
|
||
https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Update_affecteds
status-firefox59:
--- → ?
Assignee | ||
Updated•6 years ago
|
Component: Crash Reporting → IPC: MSCOM
Product: Toolkit → Core
Assignee | ||
Comment 13•6 years ago
|
||
I'm just going to remove this annotation for now. If we really need it, we can resurrect it using a different mechanism.
Comment hidden (mozreview-request) |
Comment 15•6 years ago
|
||
mozreview-review |
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+
Comment 16•6 years ago
|
||
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
Comment 17•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ed9e9171bf0f
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Comment 18•6 years ago
|
||
Doesn't seem worth backporting to Beta, but feel free to nominate it if you feel strongly otherwise.
status-firefox-esr52:
--- → unaffected
Flags: needinfo?(aklotz)
You need to log in
before you can comment on or make changes to this bug.
Description
•