Closed
Bug 1089833
Opened 10 years ago
Closed 10 years ago
Delete reply in MessageChannel::DispatchSyncMessage and DispatchInterruptMessage if channel isn't connected
Categories
(Core :: IPC, defect)
Core
IPC
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: mccr8, Assigned: mccr8)
References
Details
(Keywords: memory-leak, Whiteboard: [MemShrink:P2])
Attachments
(1 file)
3.90 KB,
patch
|
billm
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
Bill noticed this when we were looking at the code associated with the stacks in bug 1036562. Maybe this will fix it.
Assignee | ||
Updated•10 years ago
|
Whiteboard: [MemShrink]
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → continuation
Assignee | ||
Comment 1•10 years ago
|
||
MessageChannel::DispatchInterruptMessage has the same issue.
Assignee | ||
Updated•10 years ago
|
Summary: Delete reply in MessageChannel::DispatchSyncMessage if channel isn't connected → Delete reply in MessageChannel::DispatchSyncMessage and DispatchInterruptMessage if channel isn't connected
Assignee | ||
Comment 2•10 years ago
|
||
It looks like this bug is fairly old, pre-CPOWs. I converted this to nsAutoPtr<>, because that will be less error-prone. I filed bug 1090374 for making mLink an nsAutoPtr, which will fix the last instance of "delete" in MessageChannel.cpp. try run for an older version of the patch that did not use autoptr: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=241113afe6c5 bc1 did not hit the leak in 20 or so retriggers, so I think there's a decent chance this is the fix.
Attachment #8512796 -
Flags: review?(wmccloskey)
Attachment #8512796 -
Flags: review?(wmccloskey) → review+
Assignee | ||
Comment 4•10 years ago
|
||
tree's open https://hg.mozilla.org/integration/mozilla-inbound/rev/d2c6c62f63bf
Keywords: checkin-needed
Updated•10 years ago
|
Whiteboard: [MemShrink] → [MemShrink:P2]
Comment 5•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d2c6c62f63bf
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Assignee | ||
Comment 6•10 years ago
|
||
Comment on attachment 8512796 [details] [diff] [review] Delete reply in MessageChannel::DispatchSyncMessage and DispatchInterruptMessage if channel isn't connected. Approval Request Comment [Feature/regressing bug #]: old [User impact if declined]: intermittent memory leaks on e10s and B2G [Describe test coverage new/current, TBPL]: TBPL [Risks and why]: low [String/UUID change made/needed]: none
Attachment #8512796 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 8•10 years ago
|
||
Yeah, it probably affects everything, but I wasn't sure how worthwhile it would be to put on beta given that it shouldn't affect non-e10s desktop. Plus it doesn't seem to happen too much on TBPL outside of trunk. Is there some B2G branch that will be forking off beta?
Flags: needinfo?(continuation)
Comment 9•10 years ago
|
||
b2g34 takes merges from beta34 :)
Assignee | ||
Comment 10•10 years ago
|
||
Comment on attachment 8512796 [details] [diff] [review] Delete reply in MessageChannel::DispatchSyncMessage and DispatchInterruptMessage if channel isn't connected. Ah, cool, sounds like a good reason to backport. Approval Request Comment [Feature/regressing bug #]: old [User impact if declined]: B2G leaks [Describe test coverage new/current, TBPL]: TBPL tests [Risks and why]: low [String/UUID change made/needed]: none
Attachment #8512796 -
Flags: approval-mozilla-beta?
Comment 11•10 years ago
|
||
Comment on attachment 8512796 [details] [diff] [review] Delete reply in MessageChannel::DispatchSyncMessage and DispatchInterruptMessage if channel isn't connected. Approving the low-risk backport because of b2g branching.
Attachment #8512796 -
Flags: approval-mozilla-beta?
Attachment #8512796 -
Flags: approval-mozilla-beta+
Attachment #8512796 -
Flags: approval-mozilla-aurora?
Attachment #8512796 -
Flags: approval-mozilla-aurora+
Comment 12•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/b978e327eccb https://hg.mozilla.org/releases/mozilla-beta/rev/926c3f3f1f3a
Comment 13•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/926c3f3f1f3a
status-b2g-v2.1:
--- → fixed
status-b2g-v2.2:
--- → fixed
Comment 14•10 years ago
|
||
Is this something that might even be wanted for b2g32 (v2.0)?
Assignee | ||
Comment 15•10 years ago
|
||
It doesn't seem to worthwhile to me. We occasionally get a leak of 48 bytes on one test, so it doesn't seem to be all that frequent, though I suppose b2g could be different.
You need to log in
before you can comment on or make changes to this bug.
Description
•