- Revision
- 270127
- Author
- [email protected]
- Date
- 2020-11-20 12:48:22 -0800 (Fri, 20 Nov 2020)
Log 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):
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (270126 => 270127)
--- trunk/LayoutTests/ChangeLog 2020-11-20 19:58:40 UTC (rev 270126)
+++ trunk/LayoutTests/ChangeLog 2020-11-20 20:48:22 UTC (rev 270127)
@@ -1,3 +1,18 @@
+2020-11-20 Chris Dumez <[email protected]>
+
+ 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 <[email protected]>
canvas: drawImage should not raise IndexSizeError on empty sources
Added: trunk/LayoutTests/http/wpt/webaudio/the-audio-api/the-audioworklet-interface/context-already-rendering-expected.txt (0 => 270127)
--- trunk/LayoutTests/http/wpt/webaudio/the-audio-api/the-audioworklet-interface/context-already-rendering-expected.txt (rev 0)
+++ trunk/LayoutTests/http/wpt/webaudio/the-audio-api/the-audioworklet-interface/context-already-rendering-expected.txt 2020-11-20 20:48:22 UTC (rev 270127)
@@ -0,0 +1,3 @@
+
+PASS Passes if it does not crash
+
Added: trunk/LayoutTests/http/wpt/webaudio/the-audio-api/the-audioworklet-interface/context-already-rendering.html (0 => 270127)
--- trunk/LayoutTests/http/wpt/webaudio/the-audio-api/the-audioworklet-interface/context-already-rendering.html (rev 0)
+++ trunk/LayoutTests/http/wpt/webaudio/the-audio-api/the-audioworklet-interface/context-already-rendering.html 2020-11-20 20:48:22 UTC (rev 270127)
@@ -0,0 +1,32 @@
+<!doctype html>
+<title>Make sure we don't crash if the AudioWorklet gets constructed while the AudioContext is already rendering.</title>
+<script src=""
+<script src=""
+<script>
+var context;
+promise_setup(async (t) => {
+ context = new AudioContext();
+ internals.withUserGesture(() => {
+ context.resume();
+ });
+
+ const filePath = 'processors/basic-processor.js';
+ await context.audioWorklet.addModule(filePath);
+});
+
+const wait_for_audioworklet_rendering = async (node) => {
+ await new Promise((resolve) => {
+ node.port._onmessage_ = resolve;
+ });
+};
+
+promise_test(async (t) => {
+ const options = {
+ numberOfInputs: 0,
+ numberOfOutputs: 1
+ };
+
+ const node = new AudioWorkletNode(context, 'basic-processor', options);
+ await wait_for_audioworklet_rendering(node);
+}, 'Passes if it does not crash');
+</script>
Added: trunk/LayoutTests/http/wpt/webaudio/the-audio-api/the-audioworklet-interface/processors/basic-processor.js (0 => 270127)
--- trunk/LayoutTests/http/wpt/webaudio/the-audio-api/the-audioworklet-interface/processors/basic-processor.js (rev 0)
+++ trunk/LayoutTests/http/wpt/webaudio/the-audio-api/the-audioworklet-interface/processors/basic-processor.js 2020-11-20 20:48:22 UTC (rev 270127)
@@ -0,0 +1,8 @@
+class BasicProcessor extends AudioWorkletProcessor {
+ process(inputs, outputs) {
+ this.port.postMessage("did rendering");
+ return false;
+ }
+}
+
+registerProcessor('basic-processor', BasicProcessor);
Modified: trunk/Source/WebCore/ChangeLog (270126 => 270127)
--- trunk/Source/WebCore/ChangeLog 2020-11-20 19:58:40 UTC (rev 270126)
+++ trunk/Source/WebCore/ChangeLog 2020-11-20 20:48:22 UTC (rev 270127)
@@ -1,3 +1,43 @@
+2020-11-20 Chris Dumez <[email protected]>
+
+ 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 <[email protected]>
canvas: drawImage should not raise IndexSizeError on empty sources
Modified: trunk/Source/WebCore/Modules/webaudio/AudioDestinationNode.h (270126 => 270127)
--- trunk/Source/WebCore/Modules/webaudio/AudioDestinationNode.h 2020-11-20 19:58:40 UTC (rev 270126)
+++ trunk/Source/WebCore/Modules/webaudio/AudioDestinationNode.h 2020-11-20 20:48:22 UTC (rev 270127)
@@ -59,6 +59,7 @@
virtual void resume(CompletionHandler<void(Optional<Exception>&&)>&& completionHandler) { completionHandler(WTF::nullopt); }
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; }
void isPlayingDidChange() override;
Modified: trunk/Source/WebCore/Modules/webaudio/AudioWorkletNode.cpp (270126 => 270127)
--- trunk/Source/WebCore/Modules/webaudio/AudioWorkletNode.cpp 2020-11-20 19:58:40 UTC (rev 270126)
+++ trunk/Source/WebCore/Modules/webaudio/AudioWorkletNode.cpp 2020-11-20 20:48:22 UTC (rev 270127)
@@ -182,6 +182,7 @@
if (processor) {
auto locker = holdLock(m_processLock);
m_processor = WTFMove(processor);
+ m_workletThread = &Thread::current();
} else
fireProcessorErrorOnMainThread(ProcessorError::ConstructorError);
}
@@ -191,7 +192,7 @@
ASSERT(!isMainThread());
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)
output(i)->bus()->zero();
Modified: trunk/Source/WebCore/Modules/webaudio/AudioWorkletNode.h (270126 => 270127)
--- trunk/Source/WebCore/Modules/webaudio/AudioWorkletNode.h 2020-11-20 19:58:40 UTC (rev 270126)
+++ trunk/Source/WebCore/Modules/webaudio/AudioWorkletNode.h 2020-11-20 20:48:22 UTC (rev 270127)
@@ -87,6 +87,7 @@
Lock m_processLock;
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
// to pass them to AudioWorkletProcessor.
Modified: trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.cpp (270126 => 270127)
--- trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.cpp 2020-11-20 19:58:40 UTC (rev 270126)
+++ trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.cpp 2020-11-20 20:48:22 UTC (rev 270127)
@@ -1040,7 +1040,10 @@
void BaseAudioContext::addAudioParamDescriptors(const String& processorName, Vector<AudioParamDescriptor>&& descriptors)
{
ASSERT(!m_parameterDescriptorMap.contains(processorName));
+ bool wasEmpty = m_parameterDescriptorMap.isEmpty();
m_parameterDescriptorMap.add(processorName, WTFMove(descriptors));
+ if (wasEmpty)
+ workletIsReady();
}
void BaseAudioContext::sourceNodeWillBeginPlayback(AudioNode& node)
@@ -1055,6 +1058,16 @@
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();
+}
+
#if !RELEASE_LOG_DISABLED
WTFLogChannel& BaseAudioContext::logChannel() const
{
Modified: trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.h (270126 => 270127)
--- trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.h 2020-11-20 19:58:40 UTC (rev 270126)
+++ trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.h 2020-11-20 20:48:22 UTC (rev 270127)
@@ -317,6 +317,7 @@
void clear();
void scheduleNodeDeletion();
+ void workletIsReady();
// When source nodes begin playing, the BaseAudioContext keeps them alive inside m_referencedSourceNodes.
// When the nodes stop playing, they get added to m_finishedSourceNodes. After each rendering quantum,
Modified: trunk/Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.cpp (270126 => 270127)
--- trunk/Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.cpp 2020-11-20 19:58:40 UTC (rev 270126)
+++ trunk/Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.cpp 2020-11-20 20:48:22 UTC (rev 270127)
@@ -75,6 +75,7 @@
return;
ALWAYS_LOG(LOGIDENTIFIER);
+ m_wasDestinationStarted = false;
m_destination->stop();
m_destination = nullptr;
m_numberOfInputChannels = 0;
@@ -128,6 +129,7 @@
completionHandler(success ? WTF::nullopt : makeOptional(Exception { InvalidStateError, "Failed to start the audio device"_s }));
};
+ m_wasDestinationStarted = true;
m_destination->start(dispatchToRenderThreadFunction(), WTFMove(innerCompletionHandler));
}
@@ -140,6 +142,7 @@
});
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 }));
});
@@ -155,11 +158,21 @@
return;
}
+ 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());
+}
+
void DefaultAudioDestinationNode::close(CompletionHandler<void()>&& completionHandler)
{
ASSERT(isInitialized());
@@ -191,9 +204,11 @@
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());
}
return { };
Modified: trunk/Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.h (270126 => 270127)
--- trunk/Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.h 2020-11-20 19:58:40 UTC (rev 270126)
+++ trunk/Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.h 2020-11-20 20:48:22 UTC (rev 270127)
@@ -59,11 +59,13 @@
void enableInput(const String& inputDeviceId) final;
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;
bool isPlaying() final;
RefPtr<AudioDestination> m_destination;
+ bool m_wasDestinationStarted { false };
String m_inputDeviceId;
unsigned m_numberOfInputChannels { 0 };
};
