Closed Bug 1109802 Opened 9 years ago Closed 9 years ago

cubeb wasapi leaks IAudioStreamVolume

Categories

(Core :: Audio/Video, defect)

x86_64
Windows 8.1
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla37
Tracking Status
firefox35 --- fixed
firefox36 --- fixed
firefox37 --- fixed

People

(Reporter: reid.faiv, Assigned: kinetik)

References

Details

Attachments

(1 file, 1 obsolete file)

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
Thanks for reporting this!
Assignee: nobody → kinetik
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Looking at diff - there seems to be a mistake. It should be:

SafeRelease(audio_stream_volume);

not SafeRelease(stm->audio_stream_client);
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)
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.
(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!
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?
https://hg.mozilla.org/mozilla-central/rev/ca68c68e4835
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Attachment #8534848 - Flags: approval-mozilla-beta?
Attachment #8534848 - Flags: approval-mozilla-beta+
Attachment #8534848 - Flags: approval-mozilla-aurora?
Attachment #8534848 - Flags: approval-mozilla-aurora+
Depends on: 1123768
Blocks: 1027713
You need to log in before you can comment on or make changes to this bug.