Bug 219209

Summary: AudioWorkletProcessor::process() may get called on non-audio worklet thread and crash
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, philipj, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch ews-feeder: commit-queue-

Description Chris Dumez 2020-11-20 09:43:36 PST
AudioWorkletProcessor::process() may get called on non-audio worklet thread and crash:

Thread 13 Crashed:: com.apple.audio.IOThread.client
0   com.apple.JavaScriptCore      	0x000000056c733bae WTFCrash + 14 (Assertions.cpp:295)
1   com.apple.WebCore             	0x000000054e83c77b WTFCrashWithInfo(int, char const*, char const*, int) + 27 (Assertions.h:671)
2   com.apple.WebCore             	0x0000000550b22ab6 WebCore::AudioWorkletProcessor::process(WTF::Vector<WTF::RefPtr<WebCore::AudioBus, WTF::RawPtrTraits<WebCore::AudioBus>, WTF::DefaultRefDerefTraits<WebCore::AudioBus> >, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WTF::Vector<WTF::Ref<WebCore::AudioBus, WTF::RawPtrTraits<WebCore::AudioBus> >, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&, WTF::HashMap<WTF::String, std::__1::unique_ptr<WebCore::AudioArray<float>, std::__1::default_delete<WebCore::AudioArray<float> > >, WTF::DefaultHash<WTF::String>, WTF::HashTraits<WTF::String>, WTF::HashTraits<std::__1::unique_ptr<WebCore::AudioArray<float>, std::__1::default_delete<WebCore::AudioArray<float> > > > > const&, bool&) + 374 (AudioWorkletProcessor.cpp:257)
3   com.apple.WebCore             	0x0000000550b22571 WebCore::AudioWorkletNode::process(unsigned long) + 1489 (AudioWorkletNode.cpp:222)
4   com.apple.WebCore             	0x0000000550ace01e WebCore::AudioNode::processIfNecessary(unsigned long) + 462 (AudioNode.cpp:474)
5   com.apple.WebCore             	0x0000000550b2be82 WebCore::BaseAudioContext::processAutomaticPullNodes(unsigned long) + 194 (BaseAudioContext.cpp:904)
6   com.apple.WebCore             	0x0000000550ac84a6 WebCore::AudioDestinationNode::render(WebCore::AudioBus*, WebCore::AudioBus*, unsigned long, WebCore::AudioIOPosition const&) + 566 (AudioDestinationNode.cpp:102)
7   com.apple.WebCore             	0x00000005526d1939 WebCore::AudioDestinationCocoa::provideInput(WebCore::AudioBus*, unsigned long) + 185 (AudioDestinationCocoa.cpp:229)
8   com.apple.WebCore             	0x00000005526aeea5 WebCore::MultiChannelResampler::ChannelProvider::provideInput(WebCore::AudioBus*, unsigned long) + 309 (MultiChannelResampler.cpp:70)
9   com.apple.WebCore             	0x00000005526ca72d WebCore::SincResampler::consumeSource(float*, unsigned int) + 349 (SincResampler.cpp:203)
10  com.apple.WebCore             	0x00000005526cb4e8 WebCore::SincResampler::process(WebCore::AudioSourceProvider*, float*, unsigned long) + 3240 (SincResampler.cpp:524)
11  com.apple.WebCore             	0x00000005526a69e7 WebCore::MultiChannelResampler::process(WebCore::AudioSourceProvider*, WebCore::AudioBus*, unsigned long) + 279 (MultiChannelResampler.cpp:120)
12  com.apple.WebCore             	0x00000005526d174a WebCore::AudioDestinationCocoa::renderOnRenderingThead(unsigned long) + 154 (AudioDestinationCocoa.cpp:217)
13  com.apple.WebCore             	0x00000005526d1440 WebCore::AudioDestinationCocoa::render(double, unsigned long long, unsigned int, AudioBufferList*) + 528 (AudioDestinationCocoa.cpp:207)
14  com.apple.WebCore             	0x00000005526d1a54 WebCore::AudioOutputUnitAdaptor::inputProc(void*, unsigned int*, AudioTimeStamp const*, unsigned int, unsigned int, AudioBufferList*) + 132 (AudioOutputUnitAdaptor.cpp:69)

Reported by @JackSchaedler on Twitter.
Comment 1 Chris Dumez 2020-11-20 10:05:06 PST
Created attachment 414692 [details]
Patch
Comment 2 Geoffrey Garen 2020-11-20 12:33:54 PST
Comment on attachment 414692 [details]
Patch

r=me

Does this mean we can remove the thread check before we take the JS lock?
Comment 3 Chris Dumez 2020-11-20 12:36:40 PST
(In reply to Geoffrey Garen from comment #2)
> Comment on attachment 414692 [details]
> Patch
> 
> r=me
> 
> Does this mean we can remove the thread check before we take the JS lock?

I will double check but I do not believe so. Until the worklet is ready, handlePreRenderTask() and process() will get called on a non-worklet thread.
Comment 4 EWS 2020-11-20 12:48:46 PST
Committed r270127: <https://trac.webkit.org/changeset/270127>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 414692 [details].
Comment 5 Radar WebKit Bug Importer 2020-11-20 12:49:16 PST
<rdar://problem/71638769>