Avoid heap allocations under BiquadDSPKernel::updateCoefficientsIfNecessary() since this runs on the audio thread.
Thread 10 Crashed:: offline renderer 0 com.apple.JavaScriptCore 0x00000007b7d88aee 0x7b7d86000 + 10990 1 com.apple.JavaScriptCore 0x00000007b952629b 0x7b7d86000 + 24773275 2 com.apple.JavaScriptCore 0x00000007b7dc6a5f 0x7b7d86000 + 264799 3 com.apple.WebCore 0x000000079abfe795 WebCore::AudioArray<float>::resize(WTF::Checked<unsigned long, WTF::CrashOnOverflow>) + 133 (AudioArray.h:65) 4 com.apple.WebCore 0x000000079abfe701 WebCore::AudioArray<float>::AudioArray(unsigned long) + 65 (AudioArray.h:45) 5 com.apple.WebCore 0x000000079abd7b2d WebCore::AudioArray<float>::AudioArray(unsigned long) + 29 (AudioArray.h:44) 6 com.apple.WebCore 0x000000079ac41bb1 WebCore::BiquadDSPKernel::updateCoefficientsIfNecessary(unsigned long) + 129 (BiquadDSPKernel.cpp:45) 7 com.apple.WebCore 0x000000079ac42571 WebCore::BiquadDSPKernel::process(float const*, float*, unsigned long) + 161 (BiquadDSPKernel.cpp:140) 8 com.apple.WebCore 0x000000079ac43789 WebCore::BiquadProcessor::process(WebCore::AudioBus const*, WebCore::AudioBus*, unsigned long) + 233 (BiquadProcessor.cpp:106) 9 com.apple.WebCore 0x000000079abc0f3d WebCore::AudioBasicProcessorNode::process(unsigned long) + 253 (AudioBasicProcessorNode.cpp:86) 10 com.apple.WebCore 0x000000079abdc5be WebCore::AudioNode::processIfNecessary(unsigned long) + 462 (AudioNode.cpp:474) 11 com.apple.WebCore 0x000000079abdee77 WebCore::AudioNodeOutput::pull(WebCore::AudioBus*, unsigned long) + 407 (AudioNodeOutput.cpp:124) 12 com.apple.WebCore 0x000000079abdec46 WebCore::AudioNodeInput::sumAllConnections(WebCore::AudioBus*, unsigned long) + 566 (AudioNodeInput.cpp:197) 13 com.apple.WebCore 0x000000079abd7477 WebCore::AudioNodeInput::pull(WebCore::AudioBus*, unsigned long) + 295 (AudioNodeInput.cpp:225) 14 com.apple.WebCore 0x000000079abd7015 WebCore::AudioDestinationNode::render(WebCore::AudioBus*, WebCore::AudioBus*, unsigned long, WebCore::AudioIOPosition const&) + 469 (AudioDestinationNode.cpp:94) 15 com.apple.WebCore 0x000000079ac9e11d WebCore::OfflineAudioDestinationNode::offlineRender() + 877 (OfflineAudioDestinationNode.cpp:164) 16 com.apple.WebCore 0x000000079acbe213 WebCore::OfflineAudioDestinationNode::startRendering(WTF::CompletionHandler<void (WTF::Optional<WebCore::Exception>&&)>&&)::$_2::operator()() + 35 (OfflineAudioDestinationNode.cpp:103) 17 com.apple.WebCore 0x000000079acbfbee WTF::Detail::CallableWrapper<WebCore::OfflineAudioDestinationNode::startRendering(WTF::CompletionHandler<void (WTF::Optional<WebCore::Exception>&&)>&&)::$_2, void>::call() + 30 (Function.h:52)
Created attachment 423603 [details] Patch
Comment on attachment 423603 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423603&action=review > Source/WebCore/Modules/webaudio/BiquadDSPKernel.cpp:61 > + RELEASE_ASSERT(framesToProcess <= AudioUtilities::renderQuantumSize); Is this change intentional?
Comment on attachment 423603 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423603&action=review >> Source/WebCore/Modules/webaudio/BiquadDSPKernel.cpp:61 >> + RELEASE_ASSERT(framesToProcess <= AudioUtilities::renderQuantumSize); > > Is this change intentional? I had assumed it was, and I think it’s a good idea to check this at runtime unconditionally.
Comment on attachment 423603 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423603&action=review > Source/WebCore/Modules/webaudio/BiquadDSPKernel.cpp:41 > +static bool hasConstantValue(float* values, size_t framesToProcess) Could consider returning false if framesToProcess is 0, rather than reading an uninitialized value and then returning true.
(In reply to Darin Adler from comment #4) > Comment on attachment 423603 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=423603&action=review > > >> Source/WebCore/Modules/webaudio/BiquadDSPKernel.cpp:61 > >> + RELEASE_ASSERT(framesToProcess <= AudioUtilities::renderQuantumSize); > > > > Is this change intentional? > > I had assumed it was, and I think it’s a good idea to check this at runtime > unconditionally. Yes, definitely intentional as we we get a security bug if this assertion did not hold.
(In reply to Darin Adler from comment #5) > Comment on attachment 423603 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=423603&action=review > > > Source/WebCore/Modules/webaudio/BiquadDSPKernel.cpp:41 > > +static bool hasConstantValue(float* values, size_t framesToProcess) > > Could consider returning false if framesToProcess is 0, rather than reading > an uninitialized value and then returning true. Will make this change before landing.
Created attachment 423626 [details] Patch
Committed r274664: <https://commits.webkit.org/r274664> All reviewed patches have been landed. Closing bug and clearing flags on attachment 423626 [details].
<rdar://problem/75584924>