Closed
Bug 1100360
Opened 10 years ago
Closed 9 years ago
Intermittent leakcheck | default process: 32 bytes leaked (ChromeHangAnnotations, nsStringBuffer) when Gecko 36 merges to beta
Categories
(Core Graveyard :: Plug-ins, defect)
Core Graveyard
Plug-ins
Tracking
(firefox35 unaffected, firefox36+ verified, firefox37 verified, firefox-esr31 unaffected)
VERIFIED
FIXED
mozilla37
Tracking | Status | |
---|---|---|
firefox35 | --- | unaffected |
firefox36 | + | verified |
firefox37 | --- | verified |
firefox-esr31 | --- | unaffected |
People
(Reporter: RyanVM, Assigned: bugzilla)
Details
(Keywords: intermittent-failure, memory-leak)
Attachments
(1 file, 1 obsolete file)
13.55 KB,
patch
|
vladan
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
[Tracking Requested - why for this release]: Intermittent memory leaks when Gecko 36 merges to beta. Sorry, I ran into this earlier and apparently forgot to file it. This leak shows up frequently on Windows mochitest-3 and mochitest-other when I push current m-c to Try with the necessary bits set to simulate it being on the Beta release channel. Given the timing and the leak stack, I wonder if this is related to bug 818307 and/or bug 1087410. The affected test suites do correlate to plug-in related tests and the timing appears to match up. Also, e10s is disabled on !Trunk, so I'm wondering if that's relevant. https://treeherder.mozilla.org/ui/logviewer.html#?job_id=3175661&repo=try 11:02:15 INFO - TEST-INFO | leakcheck | default process: leaked 1 ChromeHangAnnotations (16 bytes) 11:02:15 INFO - TEST-INFO | leakcheck | default process: leaked 2 nsStringBuffer (16 bytes) 11:02:15 WARNING - TEST-UNEXPECTED-FAIL | leakcheck | default process: 32 bytes leaked (ChromeHangAnnotations, nsStringBuffer) I can provide the full patchset if needed, but it may be as simple as changing the version number from 36.0a1 to 36.0.
Reporter | ||
Updated•10 years ago
|
Summary: Intermittent leakcheck | default process: 32 bytes leaked (ChromeHangAnnotations, nsStringBuffer) → Intermittent leakcheck | default process: 32 bytes leaked (ChromeHangAnnotations, nsStringBuffer) when Gecko 36 merges to beta
Comment 1•10 years ago
|
||
mccr8's the leak expert now! :)
Comment 2•10 years ago
|
||
Just looking at the code, the issue could be that TelemetryImpl::RecordChromeHang does not free aAnnotations in the early return at the start. The code that passes in the ChromeHangAnnotations to it seems to assume that RecordChromeHang will take ownership. There aren't many places these objects are allocated, so it shouldn't be too hard to figure out where they aren't getting freed. Presumably there's some kind of chrome hang thing that is disabled on Beta but enabled for Aurora and Nightly.
Comment 3•10 years ago
|
||
Tracking because we don't want test failing in beta.
status-firefox36:
--- → affected
Reporter | ||
Comment 4•9 years ago
|
||
Any updates here? The leaks are still showing up in recent runs and the next uplift is on 12-Jan.
status-firefox37:
--- → affected
Updated•9 years ago
|
Flags: needinfo?(aklotz)
Assignee | ||
Comment 5•9 years ago
|
||
Assignee: nobody → aklotz
Status: NEW → ASSIGNED
Flags: needinfo?(aklotz)
Attachment #8542664 -
Flags: review?(vdjeric)
Comment 6•9 years ago
|
||
Comment on attachment 8542664 [details] [diff] [review] Convert chromehang annotations to use UniquePtr Review of attachment 8542664 [details] [diff] [review]: ----------------------------------------------------------------- ::: xpcom/threads/HangMonitor.cpp @@ +409,5 @@ > #ifdef REPORT_CHROME_HANGS > if (waitCount >= 2) { > uint32_t hangDuration = PR_IntervalToSeconds(now - lastTimestamp); > Telemetry::RecordChromeHang(hangDuration, stack, systemUptime, > + firefoxUptime, annotations); does this build? Don't you need a Move(annotations)?
Attachment #8542664 -
Flags: review?(vdjeric)
Assignee | ||
Comment 7•9 years ago
|
||
(In reply to Vladan Djeric (:vladan) from comment #6) > Comment on attachment 8542664 [details] [diff] [review] > Convert chromehang annotations to use UniquePtr > > Review of attachment 8542664 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: xpcom/threads/HangMonitor.cpp > @@ +409,5 @@ > > #ifdef REPORT_CHROME_HANGS > > if (waitCount >= 2) { > > uint32_t hangDuration = PR_IntervalToSeconds(now - lastTimestamp); > > Telemetry::RecordChromeHang(hangDuration, stack, systemUptime, > > + firefoxUptime, annotations); > > does this build? Don't you need a Move(annotations)? Oddly enough it does. Fixing...
Assignee | ||
Comment 8•9 years ago
|
||
Attachment #8542664 -
Attachment is obsolete: true
Attachment #8542676 -
Flags: review?(vdjeric)
Comment 9•9 years ago
|
||
Comment on attachment 8542676 [details] [diff] [review] Convert chromehang annotations to use UniquePtr (r2) Review of attachment 8542676 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/telemetry/Telemetry.h @@ +238,5 @@ > ProcessedStack &aStack, > int32_t aSystemUptime, > int32_t aFirefoxUptime, > + mozilla::UniquePtr<mozilla::HangMonitor::HangAnnotations> > + aAnnotations); space alignment
Attachment #8542676 -
Flags: review?(vdjeric) → review+
Assignee | ||
Comment 10•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/0365efa5999a
https://hg.mozilla.org/mozilla-central/rev/0365efa5999a
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Assignee | ||
Comment 12•9 years ago
|
||
Comment on attachment 8542676 [details] [diff] [review] Convert chromehang annotations to use UniquePtr (r2) Approval Request Comment [Feature/regressing bug #]: 818307 [User impact if declined]: Memory leaks [Describe test coverage new/current, TBPL]: on m-c [Risks and why]: None [String/UUID change made/needed]: None
Attachment #8542676 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
Attachment #8542676 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Reporter | ||
Updated•9 years ago
|
status-firefox35:
--- → unaffected
status-firefox-esr31:
--- → unaffected
Reporter | ||
Comment 13•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/a59bc1e935f8
Reporter | ||
Comment 14•9 years ago
|
||
No occurrences in my latest Try runs :)
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•