Bug 223230 – Avoid heap allocation under AudioSummingJunction::updateRenderingState()

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Audio

[Chris Dumez]

Avoid heap allocation under AudioSummingJunction::updateRenderingState(), since this runs on the audio thread:

Thread 9 Crashed:: offline renderer
0   com.apple.JavaScriptCore            0x00000003d7c3ff4e WTFCrash + 14 (Assertions.cpp:295)
1   com.apple.JavaScriptCore            0x00000003d93dc99b WTFCrashWithInfo(int, char const*, char const*, int) + 27 (Assertions.h:671)
2   com.apple.JavaScriptCore            0x00000003d7c7d607 WTF::fastMalloc(unsigned long) + 215 (FastMalloc.cpp:496)
3   com.apple.WebCore                   0x00000003b88bce75 WTF::FastMalloc::malloc(unsigned long) + 21 (FastMalloc.h:246)
4   com.apple.WebCore                   0x00000003babdcae2 bool WTF::VectorBufferBase<WebCore::AudioNodeOutput*, WTF::FastMalloc>::allocateBuffer<(WTF::FailureAction)0>(unsigned long) + 178 (Vector.h:301)
5   com.apple.WebCore                   0x00000003babdc931 bool WTF::Vector<WebCore::AudioNodeOutput*, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>::reserveCapacity<(WTF::FailureAction)0>(unsigned long) + 113 (Vector.h:1195)
6   com.apple.WebCore                   0x00000003babdc860 bool WTF::Vector<WebCore::AudioNodeOutput*, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>::expandCapacity<(WTF::FailureAction)0>(unsigned long) + 112 (Vector.h:1056)
7   com.apple.WebCore                   0x00000003babca944 WTF::Vector<WebCore::AudioNodeOutput*, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>::resize(unsigned long) + 148 (Vector.h:1102)
8   com.apple.WebCore                   0x00000003babca798 WebCore::AudioSummingJunction::updateRenderingState() + 280 (AudioSummingJunction.cpp:81)
9   com.apple.WebCore                   0x00000003babf5b97 WebCore::BaseAudioContext::handleDirtyAudioSummingJunctions() + 247 (BaseAudioContext.cpp:852)
10  com.apple.WebCore                   0x00000003babf5a47 WebCore::BaseAudioContext::handlePreRenderTasks(WebCore::AudioIOPosition const&) + 151 (BaseAudioContext.cpp:701)
11  com.apple.WebCore                   0x00000003bab8ddd3 WebCore::AudioDestinationNode::render(WebCore::AudioBus*, WebCore::AudioBus*, unsigned long, WebCore::AudioIOPosition const&) + 307 (AudioDestinationNode.cpp:81)
12  com.apple.WebCore                   0x00000003bac5483d WebCore::OfflineAudioDestinationNode::offlineRender() + 877 (OfflineAudioDestinationNode.cpp:164)
13  com.apple.WebCore                   0x00000003bac74939 WebCore::OfflineAudioDestinationNode::startRendering(WTF::CompletionHandler<void (WTF::Optional<WebCore::Exception>&&)>&&)::$_2::operator()() + 41 (OfflineAudioDestinationNode.cpp:104)
14  com.apple.WebCore                   0x00000003bac7630e WTF::Detail::CallableWrapper<WebCore::OfflineAudioDestinationNode::startRendering(WTF::CompletionHandler<void (WTF::Optional<WebCore::Exception>&&)>&&)::$_2, void>::call() + 30 (Function.h:52)
15  com.apple.JavaScriptCore            0x00000003d7c68552 WTF::Function<void ()>::operator()() const + 130 (Function.h:83)
16  com.apple.JavaScriptCore            0x00000003d7d1cba8 WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*) + 424 (Threading.cpp:181)
17  com.apple.JavaScriptCore            0x00000003d7d28d48 WTF::wtfThreadEntryPoint(void*) + 24 (ThreadingPOSIX.cpp:241)
18  libsystem_pthread.dylib             0x00007fff20686954 _pthread_start + 224
19  libsystem_pthread.dylib             0x00007fff206824a7 thread_start + 15

[Chris Dumez]

Created attachment 423275 [details]
Patch

[Chris Dumez]

Created attachment 423280 [details]
Patch

[Darin Adler]

Comment on attachment 423280 [details]
Patch

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

> Source/WebCore/Modules/webaudio/AudioSummingJunction.h:87
> +    Optional<Vector<AudioNodeOutput*>> m_pendingRenderingOutputs;

I don’t see why we have to use Optional here. If we used an empty vector instead, and replaced the checks for null with checks for an empty vector, I think all the code above would still be correct. We could then remove the special case for null/empty from updateRenderingState.

[Chris Dumez]

Created attachment 423417 [details]
Patch

[EWS]

Committed r274541: <https://commits.webkit.org/r274541>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 423417 [details].

[Radar WebKit Bug Importer]

<rdar://problem/75503829>