Skip to content

Commit

Permalink
chore: cherry-pick fix for 1233564 from chromium (#30636)
Browse files Browse the repository at this point in the history
* chore: cherry-pick fix for 1233564 from chromium

Protect HRTF database loader thread from access by different threads

This patch add a new mutex locker around the HRTF database loader
thread to ensure the safe exclusive access of the loader thread
and the HRTF database.

(cherry picked from commit 6811e850ee10847da16c4d5fdc0f845494586b65)

Bug: 1233564
Change-Id: Ie12b99ffe520d3747e34af387a37637a10aab38a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3068260
Auto-Submit: Hongchan Choi <hongchan@chromium.org>
Commit-Queue: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>

[modify] https://crrev.com/033f0bdcbe538c61f532e97b03cb9c092a94b413/third_party/blink/renderer/platform/audio/hrtf_database_loader.cc
[modify] https://crrev.com/033f0bdcbe538c61f532e97b03cb9c092a94b413/third_party/blink/renderer/platform/audio/hrtf_database_loader.h

* chore: update patches

Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
  • Loading branch information
zcbenz and patchup[bot] committed Aug 30, 2021
1 parent d927f1d commit e5935ea
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 0 deletions.
1 change: 1 addition & 0 deletions patches/chromium/.patches
Expand Up @@ -125,4 +125,5 @@ cherry-pick-ac9dc1235e28.patch
cherry-pick-4ce2abc17078.patch
cherry-pick-e2123a8e0943.patch
cherry-pick-1227933.patch
cherry-pick-1233564.patch
cherry-pick-1234009.patch
77 changes: 77 additions & 0 deletions patches/chromium/cherry-pick-1233564.patch
@@ -0,0 +1,77 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Hongchan Choi <hongchan@chromium.org>
Date: Mon, 9 Aug 2021 18:43:22 +0000
Subject: Protect HRTF database loader thread from access by different threads

This patch add a new mutex locker around the HRTF database loader
thread to ensure the safe exclusive access of the loader thread
and the HRTF database.

(cherry picked from commit 6811e850ee10847da16c4d5fdc0f845494586b65)

Bug: 1233564
Change-Id: Ie12b99ffe520d3747e34af387a37637a10aab38a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3068260
Auto-Submit: Hongchan Choi <hongchan@chromium.org>
Commit-Queue: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#908269}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3082114
Reviewed-by: Chris Mumford <cmumford@google.com>
Commit-Queue: Hongchan Choi <hongchan@chromium.org>
Cr-Commit-Position: refs/branch-heads/4577@{#601}
Cr-Branched-From: 761ddde228655e313424edec06497d0c56b0f3c4-refs/heads/master@{#902210}

diff --git a/third_party/blink/renderer/platform/audio/hrtf_database_loader.cc b/third_party/blink/renderer/platform/audio/hrtf_database_loader.cc
index 034ded03d11fa42f0d0f62c6a91f6e20ee5f93e1..01cb98a1116fe1eb6a13ff6345b6bdf4e136badc 100644
--- a/third_party/blink/renderer/platform/audio/hrtf_database_loader.cc
+++ b/third_party/blink/renderer/platform/audio/hrtf_database_loader.cc
@@ -86,6 +86,8 @@ void HRTFDatabaseLoader::LoadTask() {
void HRTFDatabaseLoader::LoadAsynchronously() {
DCHECK(IsMainThread());

+ MutexLocker locker(lock_);
+
// m_hrtfDatabase and m_thread should both be unset because this should be a
// new HRTFDatabaseLoader object that was just created by
// createAndLoadAsynchronouslyIfNecessary and because we haven't started
@@ -122,6 +124,10 @@ void HRTFDatabaseLoader::CleanupTask(base::WaitableEvent* sync) {
}

void HRTFDatabaseLoader::WaitForLoaderThreadCompletion() {
+ // We can lock this because this is called from either the main thread or
+ // the offline audio rendering thread.
+ MutexLocker locker(lock_);
+
if (!thread_)
return;

diff --git a/third_party/blink/renderer/platform/audio/hrtf_database_loader.h b/third_party/blink/renderer/platform/audio/hrtf_database_loader.h
index 3ce476fa68e066d6faf40011e94203f0fb778e71..a94997b4f7e06f96018187967faa524d4acfd5f6 100644
--- a/third_party/blink/renderer/platform/audio/hrtf_database_loader.h
+++ b/third_party/blink/renderer/platform/audio/hrtf_database_loader.h
@@ -64,8 +64,8 @@ class PLATFORM_EXPORT HRTFDatabaseLoader final
// must be called from the audio thread.
bool IsLoaded() { return Database(); }

- // waitForLoaderThreadCompletion() may be called more than once and is
- // thread-safe.
+ // May be called from both main and audio thread, and also can be called more
+ // than once.
void WaitForLoaderThreadCompletion();

// Returns the database or nullptr if the database doesn't yet exist. Must
@@ -87,11 +87,10 @@ class PLATFORM_EXPORT HRTFDatabaseLoader final
void LoadTask();
void CleanupTask(base::WaitableEvent*);

- // Holding a m_lock is required when accessing m_hrtfDatabase since we access
- // it from multiple threads.
+ // |lock_| MUST be held when accessing |hrtf_database_| or |thread_| because
+ // it can be accessed by multiple threads (e.g multiple AudioContexts).
Mutex lock_;
std::unique_ptr<HRTFDatabase> hrtf_database_;
-
std::unique_ptr<Thread> thread_;

float database_sample_rate_;

0 comments on commit e5935ea

Please sign in to comment.