Bug 223529

Summary: Avoid heap allocation under AudioNodeInput::disable() / AudioNodeInput::enable()
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: Web AudioAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, darin, eric.carlson, ews-watchlist, ggaren, glenn, jer.noble, peng.liu6, philipj, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=223226
Bug Depends on: 225174    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Chris Dumez 2021-03-19 11:43:28 PDT 
Avoid heap allocation under AudioNodeInput::disable() / AudioNodeInput::enable():

Thread 10 Crashed:: offline renderer
0   com.apple.JavaScriptCore            0x0000000177be026e WTFCrash + 14 (Assertions.cpp:295)
1   com.apple.JavaScriptCore            0x0000000179382cfb WTFCrashWithInfo(int, char const*, char const*, int) + 27 (Assertions.h:671)
2   com.apple.JavaScriptCore            0x0000000177c1d9ab WTF::fastMalloc(unsigned long) + 235 (FastMalloc.cpp:524)
3   com.apple.JavaScriptCore            0x0000000177c1d895 WTF::fastZeroedMalloc(unsigned long) + 21 (FastMalloc.cpp:114)
4   com.apple.WebCore                   0x0000000158917e85 WTF::FastMalloc::zeroedMalloc(unsigned long) + 21 (FastMalloc.h:284)
5   com.apple.WebCore                   0x000000015ab37c53 WTF::HashTable<WebCore::AudioNodeOutput*, WebCore::AudioNodeOutput*, WTF::IdentityExtractor, WTF::DefaultHash<WebCore::AudioNodeOutput*>, WTF::HashTraits<WebCore::AudioNodeOutput*>, WTF::HashTraits<WebCore::AudioNodeOutput*> >::allocateTable(unsigned int) + 35 (HashTable.h:1215)
6   com.apple.WebCore                   0x000000015ab3795a WTF::HashTable<WebCore::AudioNodeOutput*, WebCore::AudioNodeOutput*, WTF::IdentityExtractor, WTF::DefaultHash<WebCore::AudioNodeOutput*>, WTF::HashTraits<WebCore::AudioNodeOutput*>, WTF::HashTraits<WebCore::AudioNodeOutput*> >::rehash(unsigned int, WebCore::AudioNodeOutput**) + 74 (HashTable.h:1327)
7   com.apple.WebCore                   0x000000015ab37358 WTF::HashTable<WebCore::AudioNodeOutput*, WebCore::AudioNodeOutput*, WTF::IdentityExtractor, WTF::DefaultHash<WebCore::AudioNodeOutput*>, WTF::HashTraits<WebCore::AudioNodeOutput*>, WTF::HashTraits<WebCore::AudioNodeOutput*> >::expand(WebCore::AudioNodeOutput**) + 120 (HashTable.h:1249)
8   com.apple.WebCore                   0x000000015ab522f3 WTF::HashTableAddResult<WTF::HashTableIterator<WebCore::AudioNodeOutput*, WebCore::AudioNodeOutput*, WTF::IdentityExtractor, WTF::DefaultHash<WebCore::AudioNodeOutput*>, WTF::HashTraits<WebCore::AudioNodeOutput*>, WTF::HashTraits<WebCore::AudioNodeOutput*> > > WTF::HashTable<WebCore::AudioNodeOutput*, WebCore::AudioNodeOutput*, WTF::IdentityExtractor, WTF::DefaultHash<WebCore::AudioNodeOutput*>, WTF::HashTraits<WebCore::AudioNodeOutput*>, WTF::HashTraits<WebCore::AudioNodeOutput*> >::add<WTF::IdentityHashTranslator<WTF::HashTraits<WebCore::AudioNodeOutput*>, WTF::DefaultHash<WebCore::AudioNodeOutput*> >, WebCore::AudioNodeOutput* const&, WebCore::AudioNodeOutput* const&>(WebCore::AudioNodeOutput* const&, WebCore::AudioNodeOutput* const&) + 115 (HashTable.h:934)
9   com.apple.WebCore                   0x000000015ab5226a WTF::HashTable<WebCore::AudioNodeOutput*, WebCore::AudioNodeOutput*, WTF::IdentityExtractor, WTF::DefaultHash<WebCore::AudioNodeOutput*>, WTF::HashTraits<WebCore::AudioNodeOutput*>, WTF::HashTraits<WebCore::AudioNodeOutput*> >::add(WebCore::AudioNodeOutput* const&) + 74 (HashTable.h:466)
10  com.apple.WebCore                   0x000000015ab3223b WTF::HashSet<WebCore::AudioNodeOutput*, WTF::DefaultHash<WebCore::AudioNodeOutput*>, WTF::HashTraits<WebCore::AudioNodeOutput*> >::add(WebCore::AudioNodeOutput* const&) + 43 (HashSet.h:239)
11  com.apple.WebCore                   0x000000015ab3217a WebCore::AudioNodeInput::disable(WebCore::AudioNodeOutput*) + 298 (AudioNodeInput.cpp:97)
12  com.apple.WebCore                   0x000000015ab30eda WebCore::AudioNodeOutput::disable() + 346 (AudioNodeOutput.cpp:230)
13  com.apple.WebCore                   0x000000015ab30d62 WebCore::AudioNode::disableOutputs() + 98 (AudioNode.cpp:553)
14  com.apple.WebCore                   0x000000015ab30cf1 WebCore::AudioNode::disableOutputsIfNecessary() + 97 (AudioNode.cpp:545)
15  com.apple.WebCore                   0x000000015ab31228 WebCore::AudioNode::decrementConnectionCountWithLock() + 312 (AudioNode.cpp:616)
16  com.apple.WebCore                   0x000000015ab31014 WebCore::AudioNode::decrementConnectionCount() + 132 (AudioNode.cpp:587)
17  com.apple.WebCore                   0x000000015ab4f17e WebCore::AudioNodeConnectionRefDerefTraits<WebCore::AudioNode>::derefIfNotNull(WebCore::AudioNode*) + 46 (AudioNode.h:284)
18  com.apple.WebCore                   0x000000015ab4f149 WTF::RefPtr<WebCore::AudioNode, WTF::RawPtrTraits<WebCore::AudioNode>, WebCore::AudioNodeConnectionRefDerefTraits<WebCore::AudioNode> >::~RefPtr() + 41 (RefPtr.h:73)
19  com.apple.WebCore                   0x000000015ab4f115 WTF::RefPtr<WebCore::AudioNode, WTF::RawPtrTraits<WebCore::AudioNode>, WebCore::AudioNodeConnectionRefDerefTraits<WebCore::AudioNode> >::~RefPtr() + 21 (RefPtr.h:73)
20  com.apple.WebCore                   0x000000015ab8c8d7 unsigned int WTF::Vector<WTF::RefPtr<WebCore::AudioNode, WTF::RawPtrTraits<WebCore::AudioNode>, WebCore::AudioNodeConnectionRefDerefTraits<WebCore::AudioNode> >, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>::removeAllMatching<bool (WTF::RefPtr<WebCore::AudioNode, WTF::RawPtrTraits<WebCore::AudioNode>, WebCore::AudioNodeConnectionRefDerefTraits<WebCore::AudioNode> > const&)>(bool  const(&)(WTF::RefPtr<WebCore::AudioNode, WTF::RawPtrTraits<WebCore::AudioNode>, WebCore::AudioNodeConnectionRefDerefTraits<WebCore::AudioNode> > const&), unsigned long) + 263 (Vector.h:1512)
21  com.apple.WebCore                   0x000000015ab89cf6 WebCore::BaseAudioContext::derefFinishedSourceNodes() + 278 (BaseAudioContext.cpp:586)
22  com.apple.WebCore                   0x000000015ab8d77c WebCore::BaseAudioContext::handlePostRenderTasks() + 156 (BaseAudioContext.cpp:734)
23  com.apple.WebCore                   0x000000015ab2ad00 WebCore::AudioDestinationNode::render(WebCore::AudioBus*, WebCore::AudioBus*, unsigned long, WebCore::AudioIOPosition const&) + 576 (AudioDestinationNode.cpp:107)
24  com.apple.WebCore                   0x000000015abe66fd WebCore::OfflineAudioDestinationNode::offlineRender() + 877 (OfflineAudioDestinationNode.cpp:163)
Comment 1 Chris Dumez 2021-03-19 12:56:56 PDT 
Created attachment 423762 [details]
Patch
Comment 2 Chris Dumez 2021-03-19 13:39:06 PDT 
Created attachment 423769 [details]
Patch
Comment 3 Peng Liu 2021-03-19 14:30:36 PDT 
Comment on attachment 423769 [details]
Patch

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

> Source/WebCore/Modules/webaudio/AudioSummingJunction.cpp:135
> +        m_pendingRenderingOutputs->updatedEnabledState(output);

Nit. Regarding "m_pendingRenderingOutputs." and "m_pendingRenderingOutputs->", I believe they are working. But they look strange at first glance. :-)

> Source/WebCore/Modules/webaudio/AudioSummingJunction.h:69
> +            bool operator==(AudioNodeOutput* other) const { return output == other; }

const AudioNodeOutput* other?
Comment 4 Chris Dumez 2021-03-19 14:44:01 PDT 
(In reply to Peng Liu from comment #3)
> Comment on attachment 423769 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=423769&action=review
> 
> > Source/WebCore/Modules/webaudio/AudioSummingJunction.cpp:135
> > +        m_pendingRenderingOutputs->updatedEnabledState(output);
> 
> Nit. Regarding "m_pendingRenderingOutputs." and
> "m_pendingRenderingOutputs->", I believe they are working. But they look
> strange at first glance. :-)

What looks strange? It is an Optional<> now so I need to `->` instead of `.`

> 
> > Source/WebCore/Modules/webaudio/AudioSummingJunction.h:69
> > +            bool operator==(AudioNodeOutput* other) const { return output == other; }
> 
> const AudioNodeOutput* other?

OK.
Comment 5 Peng Liu 2021-03-19 14:52:43 PDT 
Comment on attachment 423769 [details]
Patch

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

>>> Source/WebCore/Modules/webaudio/AudioSummingJunction.cpp:135
>>> +        m_pendingRenderingOutputs->updatedEnabledState(output);
>> 
>> Nit. Regarding "m_pendingRenderingOutputs." and "m_pendingRenderingOutputs->", I believe they are working. But they look strange at first glance. :-)
> 
> What looks strange? It is an Optional<> now so I need to `->` instead of `.`

At first glance I was surprised by the "m_pendingRenderingOutputs." in the "if" branch and "m_pendingRenderingOutputs->" in the "else" branch. But it is not an issue actually.
:-)
Comment 6 Chris Dumez 2021-03-19 15:38:54 PDT 
Created attachment 423789 [details]
Patch
Comment 7 EWS 2021-03-22 04:58:34 PDT 
commit-queue failed to commit attachment 423789 [details] to WebKit repository. To retry, please set cq+ flag again.
Comment 8 EWS 2021-03-22 11:51:48 PDT 
Committed r274767: <https://commits.webkit.org/r274767>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 423789 [details].
Comment 9 Radar WebKit Bug Importer 2021-03-22 11:52:16 PDT 
<rdar://problem/75701704>