diff --git a/patches/chromium/.patches b/patches/chromium/.patches index bbb128d9a2707..cf07a626659e8 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -129,3 +129,4 @@ merge_to_m88_xproto_switch_event_queue_from_a_std_list_to_a.patch websocket_don_t_clear_event_queue_on_destruction.patch cherry-pick-7e0e52df283c.patch cherry-pick-6ed1c0c425e0.patch +cherry-pick-aeb6bc551b60.patch diff --git a/patches/chromium/cherry-pick-aeb6bc551b60.patch b/patches/chromium/cherry-pick-aeb6bc551b60.patch new file mode 100644 index 0000000000000..508cfdf6df1c7 --- /dev/null +++ b/patches/chromium/cherry-pick-aeb6bc551b60.patch @@ -0,0 +1,114 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Hongchan Choi +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 +Reviewed-by: Raymond Toy +Cr-Original-Commit-Position: refs/heads/master@{#852240} +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2715585 +Commit-Queue: Krishna Govind +Reviewed-by: Srinivas Sista +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 waitable_event = +- std::make_unique(); +- +- 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 waitable_event = ++ std::make_unique(); ++ PostCrossThreadTask( ++ *task_runner_, FROM_HERE, ++ CrossThreadBindOnce( ++ &ScriptProcessorHandler::FireProcessEventForOfflineAudioContext, ++ AsWeakPtr(), double_buffer_index_, ++ CrossThreadUnretained(waitable_event.get()))); ++ waitable_event->Wait(); + } + + SwapBuffers();