Closed
Bug 1109802
Opened 10 years ago
Closed 10 years ago
cubeb wasapi leaks IAudioStreamVolume
Categories
(Core :: Audio/Video, defect)
Tracking
()
RESOLVED
FIXED
mozilla37
People
(Reporter: reid.faiv, Assigned: kinetik)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
padenot
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
media/libcubeb/src/cubeb_wasapi.cpp
wasapi_stream_init() function allocates IAudioStreamVolume (stm->audio_stream_volume):
hr = stm->client->GetService(__uuidof(IAudioStreamVolume),
(void **)&stm->audio_stream_volume);
but this is is not released in wasapi_stream_destroy()
according to MSDN docs caller must free it.
http://msdn.microsoft.com/en-us/library/windows/desktop/dd370873(v=vs.85).aspx
Not releasing IAudioStreamVolume causes also IAudioClient to stick around even though SafeRelease() is called on it. This is defined behavior according to MSDN
http://msdn.microsoft.com/en-us/library/windows/desktop/dd316756(v=vs.85).aspx
<quote>
Following the call to the IAudioClient::Initialize method, the stream remains open until the client releases all of its references to the IAudioClient interface and to all references to service interfaces that the client obtained through the IAudioClient::GetService method. The final Release call closes the stream.
</quote>
On my machine each IAudioClient instance consumes about 1.8MB audiodg.exe memory, which after while leads easily to quite high audiodg.exe memory consumption. This memory is released back to when Firefox is closed.
Originally I posted to bug 1107124 following sample HTML which on each "play" click causes 1.8M uptick on audiodg.exe private bytes. Try clicking 10 times.
<html>
<body>
<button onclick="play();">click here multiple times to play</button>
<button onclick="stop();">stop</button>
<audio id="a" src = "http://hpr.dogphilosophy.net/test/mp3.mp3"></audio>
<script>
function play() {
audio = document.getElementById("a");
audio.pause();
audio.currentTime = 0;
audio.play();
}
function stop() {
audio = document.getElementById("a");
audio.pause();
}
</script>
</body>
</html>
Firefox debug build log shows that AudioStream itself is disposed, so this definitely look cubeb_wasapi bug:
13868[140481d0]: mozilla::AudioStream::Init channels: 2, rate: 48000 for dacb5c0
13868[140481d0]: AudioStream creation time first: 16 ms
...
13868[140481d0]: AudioStream: StateCallback dacb5c0, mState=0 cubeb_state=0
13868[140481d0]: AudioStream: started dacb5c0, state STARTED
...
13868[140481d0]: AudioStream: StateCallback dacb5c0, mState=2 cubeb_state=1
13868[140481d0]: AudioStream: Shutdown dacb5c0, state 6
13868[140481d0]: AudioStream: StateCallback dacb5c0, mState=6 cubeb_state=1
13868[140481d0]: AudioStream: delete dacb5c0, state 7
Assignee | ||
Comment 2•10 years ago
|
||
Thanks for reporting this!
Assignee: nobody → kinetik
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee | ||
Comment 3•10 years ago
|
||
Assignee | ||
Comment 4•10 years ago
|
||
Attachment #8534654 -
Flags: review?(padenot)
Looking at diff - there seems to be a mistake. It should be:
SafeRelease(audio_stream_volume);
not SafeRelease(stm->audio_stream_client);
Assignee | ||
Comment 6•10 years ago
|
||
Comment on attachment 8534654 [details] [diff] [review]
bug1109802_v0.patch
Yeah, it failed on Try too. That's what I get for patching before coffee!
Attachment #8534654 -
Attachment is obsolete: true
Attachment #8534654 -
Flags: review?(padenot)
Assignee | ||
Comment 7•10 years ago
|
||
Attachment #8534848 -
Flags: review?(padenot)
Updated•10 years ago
|
Attachment #8534848 -
Flags: review?(padenot) → review+
tested https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mgregan@mozilla.com-9dc11d51e038/try-win32-debug/ build - looks good now, audiodg.exe memory drops back when audio.play() finishes.
Assignee | ||
Comment 9•10 years ago
|
||
Assignee | ||
Comment 10•10 years ago
|
||
(In reply to reid.faiv from comment #8)
> tested
> https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mgregan@mozilla.
> com-9dc11d51e038/try-win32-debug/ build - looks good now, audiodg.exe memory
> drops back when audio.play() finishes.
Thanks for confirming, and thanks again for reporting the bug!
Assignee | ||
Comment 11•10 years ago
|
||
Assignee | ||
Comment 12•10 years ago
|
||
Comment on attachment 8534848 [details] [diff] [review]
bug1109802_v1.patch
Approval Request Comment
[Feature/regressing bug #]: bug 1027713
[User impact if declined]: leak ~1.8MB in the OS audio server per media element/Web Audio/WebRTC output on Vista+
[Describe test coverage new/current, TBPL]: none, not easily testable
[Risks and why]: virtually none, fix is trivial
[String/UUID change made/needed]: none
Attachment #8534848 -
Flags: approval-mozilla-beta?
Attachment #8534848 -
Flags: approval-mozilla-aurora?
Updated•10 years ago
|
Comment 13•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Updated•10 years ago
|
Attachment #8534848 -
Flags: approval-mozilla-beta?
Attachment #8534848 -
Flags: approval-mozilla-beta+
Attachment #8534848 -
Flags: approval-mozilla-aurora?
Attachment #8534848 -
Flags: approval-mozilla-aurora+
Comment 14•10 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•