Changeset 270127 in webkit

Timestamp:

Nov 20, 2020 12:48:22 PM (3 years ago)

Author:

Chris Dumez

Message:

AudioWorkletProcessor::process() may get called on non-audio worklet thread and crash
​https://bugs.webkit.org/show_bug.cgi?id=219209

Reviewed by Geoffrey Garen.

Source/WebCore:

If the AudioContext is already running when the AudioWorklet becomes ready, then the
AudioWorkletNode::process() was getting called on the CoreAudio rendering thread instead
of the AudioWorklet thread. This was causing us to run the AudioWorkletProcessor's JS
on the wrong thread, which would cause flaky crashes in release and assertion hits in
debug.

To address the issue, I updated AudioWorkletNode::process() to output silence when
it is called on another thread than the AudioWorklet thread. Also, if the AudioContext
is already running when the AudioWorklet becomes ready, we need to restart the
AudioDestination so that we switch the audio rendering from the CoreAudio rendering
thread to the AudioWorklet thread.

Test: http/wpt/webaudio/the-audio-api/the-audioworklet-interface/context-already-rendering.html

• Modules/webaudio/AudioDestinationNode.h:

(WebCore::AudioDestinationNode::restartRendering):

• Modules/webaudio/AudioWorkletNode.cpp:

(WebCore::AudioWorkletNode::isWorkletThread):
(WebCore::AudioWorkletNode::process):

• Modules/webaudio/AudioWorkletNode.h:
• Modules/webaudio/BaseAudioContext.cpp:

(WebCore::BaseAudioContext::addAudioParamDescriptors):
(WebCore::BaseAudioContext::workletIsReady):

• Modules/webaudio/BaseAudioContext.h:
• Modules/webaudio/DefaultAudioDestinationNode.cpp:

(WebCore::DefaultAudioDestinationNode::uninitialize):
(WebCore::DefaultAudioDestinationNode::startRendering):
(WebCore::DefaultAudioDestinationNode::resume):
(WebCore::DefaultAudioDestinationNode::suspend):
(WebCore::DefaultAudioDestinationNode::restartRendering):
(WebCore::DefaultAudioDestinationNode::setChannelCount):

• Modules/webaudio/DefaultAudioDestinationNode.h:

LayoutTests:

Add layout test coverage.

• http/wpt/webaudio/the-audio-api/the-audioworklet-interface/context-already-rendering-expected.txt: Added.
• http/wpt/webaudio/the-audio-api/the-audioworklet-interface/context-already-rendering.html: Added.
• http/wpt/webaudio/the-audio-api/the-audioworklet-interface/processors/basic-processor.js: Added.

(BasicProcessor.prototype.process):
(BasicProcessor):

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/http/wpt/webaudio/the-audio-api/the-audioworklet-interface/context-already-rendering-expected.txt (added)
• LayoutTests/http/wpt/webaudio/the-audio-api/the-audioworklet-interface/context-already-rendering.html (added)
• LayoutTests/http/wpt/webaudio/the-audio-api/the-audioworklet-interface/processors/basic-processor.js (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/Modules/webaudio/AudioDestinationNode.h (modified) (1 diff)
• Source/WebCore/Modules/webaudio/AudioWorkletNode.cpp (modified) (2 diffs)
• Source/WebCore/Modules/webaudio/AudioWorkletNode.h (modified) (1 diff)
• Source/WebCore/Modules/webaudio/BaseAudioContext.cpp (modified) (2 diffs)
• Source/WebCore/Modules/webaudio/BaseAudioContext.h (modified) (1 diff)
• Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.cpp (modified) (5 diffs)
• Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.h (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2020-11-20  Chris Dumez  <cdumez@apple.com>
+
+        AudioWorkletProcessor::process() may get called on non-audio worklet thread and crash
+        https://bugs.webkit.org/show_bug.cgi?id=219209
+
+        Reviewed by Geoffrey Garen.
+
+        Add layout test coverage.
+
+        * http/wpt/webaudio/the-audio-api/the-audioworklet-interface/context-already-rendering-expected.txt: Added.
+        * http/wpt/webaudio/the-audio-api/the-audioworklet-interface/context-already-rendering.html: Added.
+        * http/wpt/webaudio/the-audio-api/the-audioworklet-interface/processors/basic-processor.js: Added.
+        (BasicProcessor.prototype.process):
+        (BasicProcessor):
+
 2020-11-20  Lauro Moura  <lmoura@igalia.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-11-20  Chris Dumez  <cdumez@apple.com>
+
+        AudioWorkletProcessor::process() may get called on non-audio worklet thread and crash
+        https://bugs.webkit.org/show_bug.cgi?id=219209
+
+        Reviewed by Geoffrey Garen.
+
+        If the AudioContext is already running when the AudioWorklet becomes ready, then the
+        AudioWorkletNode::process() was getting called on the CoreAudio rendering thread instead
+        of the AudioWorklet thread. This was causing us to run the AudioWorkletProcessor's JS
+        on the wrong thread, which would cause flaky crashes in release and assertion hits in
+        debug.
+
+        To address the issue, I updated AudioWorkletNode::process() to output silence when
+        it is called on another thread than the AudioWorklet thread. Also, if the AudioContext
+        is already running when the AudioWorklet becomes ready, we need to restart the
+        AudioDestination so that we switch the audio rendering from the CoreAudio rendering
+        thread to the AudioWorklet thread.
+
+        Test: http/wpt/webaudio/the-audio-api/the-audioworklet-interface/context-already-rendering.html
+
+        * Modules/webaudio/AudioDestinationNode.h:
+        (WebCore::AudioDestinationNode::restartRendering):
+        * Modules/webaudio/AudioWorkletNode.cpp:
+        (WebCore::AudioWorkletNode::isWorkletThread):
+        (WebCore::AudioWorkletNode::process):
+        * Modules/webaudio/AudioWorkletNode.h:
+        * Modules/webaudio/BaseAudioContext.cpp:
+        (WebCore::BaseAudioContext::addAudioParamDescriptors):
+        (WebCore::BaseAudioContext::workletIsReady):
+        * Modules/webaudio/BaseAudioContext.h:
+        * Modules/webaudio/DefaultAudioDestinationNode.cpp:
+        (WebCore::DefaultAudioDestinationNode::uninitialize):
+        (WebCore::DefaultAudioDestinationNode::startRendering):
+        (WebCore::DefaultAudioDestinationNode::resume):
+        (WebCore::DefaultAudioDestinationNode::suspend):
+        (WebCore::DefaultAudioDestinationNode::restartRendering):
+        (WebCore::DefaultAudioDestinationNode::setChannelCount):
+        * Modules/webaudio/DefaultAudioDestinationNode.h:
+
 2020-11-20  Lauro Moura  <lmoura@igalia.com>
 

trunk/Source/WebCore/Modules/webaudio/AudioDestinationNode.h

@@ -60 +60 @@
     virtual void suspend(CompletionHandler<void(Optional<Exception>&&)>&& completionHandler) { completionHandler(WTF::nullopt); }
     virtual void close(CompletionHandler<void()>&& completionHandler) { completionHandler(); }
+    virtual void restartRendering() { }
 
     virtual bool isPlaying() { return false; }

trunk/Source/WebCore/Modules/webaudio/AudioWorkletNode.cpp

@@ -183 +183 @@
         auto locker = holdLock(m_processLock);
         m_processor = WTFMove(processor);
+        m_workletThread = &Thread::current();
     } else
         fireProcessorErrorOnMainThread(ProcessorError::ConstructorError);
@@ -192 +193 @@
 
     auto locker = tryHoldLock(m_processLock);
-    if (!locker || !m_processor) {
+    if (!locker || !m_processor || &Thread::current() != m_workletThread) {
         // We're not ready yet or we are getting destroyed. In this case, we output silence.
         for (unsigned i = 0; i < numberOfOutputs(); ++i)

trunk/Source/WebCore/Modules/webaudio/AudioWorkletNode.h

@@ -88 +88 @@
     RefPtr<AudioWorkletProcessor> m_processor; // Should only be used on the rendering thread.
     HashMap<String, std::unique_ptr<AudioFloatArray>> m_paramValuesMap;
+    Thread* m_workletThread { nullptr };
 
     // Keeps the reference of AudioBus objects from AudioNodeInput and AudioNodeOutput in order

trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.cpp

@@ -1041 +1041 @@
 {
     ASSERT(!m_parameterDescriptorMap.contains(processorName));
+    bool wasEmpty = m_parameterDescriptorMap.isEmpty();
     m_parameterDescriptorMap.add(processorName, WTFMove(descriptors));
+    if (wasEmpty)
+        workletIsReady();
 }
 
@@ -1054 +1057 @@
 
     m_finishedSourceNodes.append(&node);
+}
+
+void BaseAudioContext::workletIsReady()
+{
+    ASSERT(isMainThread());
+
+    // If we're already rendering when the worklet becomes ready, we need to restart
+    // rendering in order to switch to the audio worklet thread.
+    if (m_destinationNode)
+        m_destinationNode->restartRendering();
 }
 

trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.h

@@ -318 +318 @@
 
     void scheduleNodeDeletion();
+    void workletIsReady();
 
     // When source nodes begin playing, the BaseAudioContext keeps them alive inside m_referencedSourceNodes.

trunk/Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.cpp

@@ -76 +76 @@
 
     ALWAYS_LOG(LOGIDENTIFIER);
+    m_wasDestinationStarted = false;
     m_destination->stop();
     m_destination = nullptr;
@@ -129 +130 @@
     };
 
+    m_wasDestinationStarted = true;
     m_destination->start(dispatchToRenderThreadFunction(), WTFMove(innerCompletionHandler));
 }
@@ -141 +143 @@
         return;
     }
+    m_wasDestinationStarted = true;
     m_destination->start(dispatchToRenderThreadFunction(), [completionHandler = WTFMove(completionHandler)](bool success) mutable {
         completionHandler(success ? WTF::nullopt : makeOptional(Exception { InvalidStateError, "Failed to start the audio device"_s }));
@@ -156 +159 @@
     }
 
+    m_wasDestinationStarted = false;
     m_destination->stop([completionHandler = WTFMove(completionHandler)](bool success) mutable {
         completionHandler(success ? WTF::nullopt : makeOptional(Exception { InvalidStateError, "Failed to stop the audio device"_s }));
     });
+}
+
+void DefaultAudioDestinationNode::restartRendering()
+{
+    if (!m_wasDestinationStarted)
+        return;
+
+    m_destination->stop();
+    m_destination->start(dispatchToRenderThreadFunction());
 }
 
@@ -192 +205 @@
     if (this->channelCount() != oldChannelCount && isInitialized()) {
         // Re-create destination.
-        m_destination->stop();
+        if (m_wasDestinationStarted)
+            m_destination->stop();
         createDestination();
-        m_destination->start(dispatchToRenderThreadFunction());
+        if (m_wasDestinationStarted)
+            m_destination->start(dispatchToRenderThreadFunction());
     }
 

trunk/Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.h

@@ -60 +60 @@
     void resume(CompletionHandler<void(Optional<Exception>&&)>&&) final;
     void suspend(CompletionHandler<void(Optional<Exception>&&)>&&) final;
+    void restartRendering() final;
     void close(CompletionHandler<void()>&&) final;
     unsigned maxChannelCount() const final;
@@ -65 +66 @@
 
     RefPtr<AudioDestination> m_destination;
+    bool m_wasDestinationStarted { false };
     String m_inputDeviceId;
     unsigned m_numberOfInputChannels { 0 };