|
|
@@ -0,0 +1,114 @@
|
|
|
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
|
+From: Hongchan Choi <[email protected]>
|
|
|
+Date: Tue, 23 Feb 2021 23:27:31 +0000
|
|
|
+Subject: Prevent accessing shared buffers from audio rendering thread
|
|
|
+
|
|
|
+The shared buffer in ScriptProcessorNode can be accessed by the
|
|
|
+audio rendering thread when it is held by the main thread.
|
|
|
+
|
|
|
+The solution suggested here is simply to expand the scope of
|
|
|
+the mutex to minimize the code change. This is a deprecated
|
|
|
+feature in Web Audio, so making significant changes is not
|
|
|
+sensible. By locking the entire scope of Process() call, this
|
|
|
+area would be immune to the similar problems in the future.
|
|
|
+
|
|
|
+(cherry picked from commit 60987aa224f369fc0ea38c56e498389440921356)
|
|
|
+
|
|
|
+Bug: 1174582
|
|
|
+Test: The repro case doesn't crash on ASAN.
|
|
|
+Change-Id: I2b292f94be65e6ec26c6eb0e0ed32b3fb2d88466
|
|
|
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2681193
|
|
|
+Commit-Queue: Hongchan Choi <[email protected]>
|
|
|
+Reviewed-by: Raymond Toy <[email protected]>
|
|
|
+Cr-Original-Commit-Position: refs/heads/master@{#852240}
|
|
|
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2715585
|
|
|
+Commit-Queue: Krishna Govind <[email protected]>
|
|
|
+Reviewed-by: Srinivas Sista <[email protected]>
|
|
|
+Cr-Commit-Position: refs/branch-heads/4324@{#2238}
|
|
|
+Cr-Branched-From: c73b5a651d37a6c4d0b8e3262cc4015a5579c6c8-refs/heads/master@{#827102}
|
|
|
+
|
|
|
+diff --git a/third_party/blink/renderer/modules/webaudio/script_processor_node.cc b/third_party/blink/renderer/modules/webaudio/script_processor_node.cc
|
|
|
+index b1ca691b07b53b927a92753906f7f25edebac919..6e80b23a32dd1895a0d51d08ee16c8cb2d44fc55 100644
|
|
|
+--- a/third_party/blink/renderer/modules/webaudio/script_processor_node.cc
|
|
|
++++ b/third_party/blink/renderer/modules/webaudio/script_processor_node.cc
|
|
|
+@@ -110,6 +110,14 @@ void ScriptProcessorHandler::Initialize() {
|
|
|
+ }
|
|
|
+
|
|
|
+ void ScriptProcessorHandler::Process(uint32_t frames_to_process) {
|
|
|
++ // The main thread might be accessing the shared buffers. If so, silience
|
|
|
++ // the output and return.
|
|
|
++ MutexTryLocker try_locker(process_event_lock_);
|
|
|
++ if (!try_locker.Locked()) {
|
|
|
++ Output(0).Bus()->Zero();
|
|
|
++ return;
|
|
|
++ }
|
|
|
++
|
|
|
+ // Discussion about inputs and outputs:
|
|
|
+ // As in other AudioNodes, ScriptProcessorNode uses an AudioBus for its input
|
|
|
+ // and output (see inputBus and outputBus below). Additionally, there is a
|
|
|
+@@ -181,47 +189,26 @@ void ScriptProcessorHandler::Process(uint32_t frames_to_process) {
|
|
|
+ buffer_read_write_index_ =
|
|
|
+ (buffer_read_write_index_ + frames_to_process) % BufferSize();
|
|
|
+
|
|
|
+- // m_bufferReadWriteIndex will wrap back around to 0 when the current input
|
|
|
+- // and output buffers are full.
|
|
|
+- // When this happens, fire an event and swap buffers.
|
|
|
++ // Fire an event and swap buffers when |buffer_read_write_index_| wraps back
|
|
|
++ // around to 0. It means the current input and output buffers are full.
|
|
|
+ if (!buffer_read_write_index_) {
|
|
|
+- // Avoid building up requests on the main thread to fire process events when
|
|
|
+- // they're not being handled. This could be a problem if the main thread is
|
|
|
+- // very busy doing other things and is being held up handling previous
|
|
|
+- // requests. The audio thread can't block on this lock, so we call
|
|
|
+- // tryLock() instead.
|
|
|
+- MutexTryLocker try_locker(process_event_lock_);
|
|
|
+- if (!try_locker.Locked()) {
|
|
|
+- // We're late in handling the previous request. The main thread must be
|
|
|
+- // very busy. The best we can do is clear out the buffer ourself here.
|
|
|
+- shared_output_buffer->Zero();
|
|
|
++ if (Context()->HasRealtimeConstraint()) {
|
|
|
++ // For a realtime context, fire an event and do not wait.
|
|
|
++ PostCrossThreadTask(
|
|
|
++ *task_runner_, FROM_HERE,
|
|
|
++ CrossThreadBindOnce(&ScriptProcessorHandler::FireProcessEvent,
|
|
|
++ AsWeakPtr(), double_buffer_index_));
|
|
|
+ } else {
|
|
|
+- // With the realtime context, execute the script code asynchronously
|
|
|
+- // and do not wait.
|
|
|
+- if (Context()->HasRealtimeConstraint()) {
|
|
|
+- // Fire the event on the main thread with the appropriate buffer
|
|
|
+- // index.
|
|
|
+- PostCrossThreadTask(
|
|
|
+- *task_runner_, FROM_HERE,
|
|
|
+- CrossThreadBindOnce(&ScriptProcessorHandler::FireProcessEvent,
|
|
|
+- AsWeakPtr(), double_buffer_index_));
|
|
|
+- } else {
|
|
|
+- // If this node is in the offline audio context, use the
|
|
|
+- // waitable event to synchronize to the offline rendering thread.
|
|
|
+- std::unique_ptr<base::WaitableEvent> waitable_event =
|
|
|
+- std::make_unique<base::WaitableEvent>();
|
|
|
+-
|
|
|
+- PostCrossThreadTask(
|
|
|
+- *task_runner_, FROM_HERE,
|
|
|
+- CrossThreadBindOnce(
|
|
|
+- &ScriptProcessorHandler::FireProcessEventForOfflineAudioContext,
|
|
|
+- AsWeakPtr(), double_buffer_index_,
|
|
|
+- CrossThreadUnretained(waitable_event.get())));
|
|
|
+-
|
|
|
+- // Okay to block the offline audio rendering thread since it is
|
|
|
+- // not the actual audio device thread.
|
|
|
+- waitable_event->Wait();
|
|
|
+- }
|
|
|
++ // For an offline context, wait until the script execution is finished.
|
|
|
++ std::unique_ptr<base::WaitableEvent> waitable_event =
|
|
|
++ std::make_unique<base::WaitableEvent>();
|
|
|
++ PostCrossThreadTask(
|
|
|
++ *task_runner_, FROM_HERE,
|
|
|
++ CrossThreadBindOnce(
|
|
|
++ &ScriptProcessorHandler::FireProcessEventForOfflineAudioContext,
|
|
|
++ AsWeakPtr(), double_buffer_index_,
|
|
|
++ CrossThreadUnretained(waitable_event.get())));
|
|
|
++ waitable_event->Wait();
|
|
|
+ }
|
|
|
+
|
|
|
+ SwapBuffers();
|
Pedro Pontes