All users were logged out of Bugzilla on October 13th, 2018

We should take Chromium fix in commit b533d6533585377edd63ec6500469f6c4fba602a to stop a memory leak.

RESOLVED FIXED in Firefox 38

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: bobowen, Assigned: bobowen)

Tracking

unspecified
mozilla39
All
Windows 7
Points:
---

Firefox Tracking Flags

(firefox36 wontfix, firefox37 wontfix, firefox38 fixed, firefox39 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
See https://codereview.chromium.org/831423002/ for details.

This may not have been biting us much, but could start to with EME coming along.
(Assignee)

Comment 1

4 years ago
Created attachment 8583238 [details] [diff] [review]
Chromium patch to fix memory leak in Windows sandbox sharedmem_ipc_server.cc.

In my not very scientific test, if I run multiple GMP tests with [1] the baseline memory certainly seems to climb.
This doesn't happen with the fix.

I took the other change that had happened in the file as well, as it makes no functional difference.

[1] http://mozilla.github.io/webrtc-landing/pc_test.html
Attachment #8583238 - Flags: review?(aklotz)

Updated

4 years ago
Attachment #8583238 - Flags: review?(aklotz) → review+
(Assignee)

Comment 2

4 years ago
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/f3cc3b904308

Thanks for the quick review Aaron, particularly given how busy you are this week.
(Assignee)

Comment 3

4 years ago
Comment on attachment 8583238 [details] [diff] [review]
Chromium patch to fix memory leak in Windows sandbox sharedmem_ipc_server.cc.

Approval Request Comment
[Feature/regressing bug #]:
This has been in the Chromium Windows sandbox code since we shipped it.
(In fact since Chromium shipped it as well.)

[User impact if declined]:
Going by the Chromium bug comments, each sandboxed process will continue to leak 64KB when it shuts down.
We are likely to be getting more GMP processes with EME shipping in Fx38.

[Describe test coverage new/current, TreeHerder]:
The only things that we ship that use the sandbox are GMPs (OpenH264 and soon to be EME) and these have mochitests.

[Risks and why]:
Low: It's a small obvious fix and is already shipping in the Chrome stable release (announced 3rd March).

[String/UUID change made/needed]:
None.
Attachment #8583238 - Flags: approval-mozilla-aurora?
Attachment #8583238 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Updated

4 years ago
status-firefox36: --- → affected
status-firefox37: --- → affected
status-firefox38: --- → affected
status-firefox39: --- → affected
https://hg.mozilla.org/mozilla-central/rev/f3cc3b904308
Status: NEW → RESOLVED
Last Resolved: 4 years ago
status-firefox39: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
status-firefox36: affected → wontfix
status-firefox37: affected → wontfix
You need to log in before you can comment on or make changes to this bug.