Bug 64157 – AudioDevice::Stop can close NULL handle.

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Berend-Jan Wever]

Created attachment 100102 [details]
Repro (press F5 to crash)

Chromium: https://code.google.com/p/chromium/issues/detail?id=88740

Repro:
<script>
  setInterval(function () {
    var oAudioContext = new webkitAudioContext();
    oGainNode = oAudioContext.createGainNode();
  }, 1);
</script>
Load the above file, then press F5 to reload and cause the crash. The code tries to join a thread whose id is 0, which leads to closing an invalid handle (0) on Windows. I don't know how linux/mac handle this.

Possible solutions:
- AudioDevice::Stop should check if the thread exists before joining it
http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/content/renderer/media/audio_device.cc&q=&exact_package=chromium&l=114

- SimpleThread::Join() should return immediately if the thread id is 0.
http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/base/threading/simple_thread.cc&q=&exact_package=chromium&l=39

[Chris Rogers]

Created attachment 100382 [details]
Patch

[Kenneth Russell]

Comment on attachment 100382 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=100382&action=review

Basically looks good except for a small issue in the bindings.

> Source/WebCore/bindings/js/JSAudioContextCustom.cpp:72
> +            return throwVMError(exec, createReferenceError(exec, "audio resources unavailable for AudioContext construction"));

The JS bindings are throwing a ReferenceError, while the V8 bindings are throwing a SyntaxError. These should be aligned.

[Chris Rogers]

Created attachment 100565 [details]
Patch

[Chris Rogers]

Sorry, this was already R+, but after some extensive testing I found that it's necessary to decrement s_hardwareContextCount in the uninitialize() method instead of destructor since garbage collection can take a long time and hold up new contexts being created.

[Kenneth Russell]

Comment on attachment 100565 [details]
Patch

OK.

[Chris Rogers]

Committed r90853: <http://trac.webkit.org/changeset/90853>