Commit 739acb48 authored by Anton Bikineev's avatar Anton Bikineev Committed by V8 LUCI CQ

cppgc: shared-heap: Fix data race around CagedHeap::large_pages_

Now that the cage is shared, its metadata must be thread-safe.

Bug: chromium:1336529
Change-Id: I0650462d1faf171fc3325808ca45ebe044e91f45
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3707097
Auto-Submit: Anton Bikineev <bikineev@chromium.org>
Commit-Queue: Anton Bikineev <bikineev@chromium.org>
Reviewed-by: 's avatarMichael Lippautz <mlippautz@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81176}
parent f789c6a0
...@@ -148,6 +148,7 @@ CagedHeap::CagedHeap(PageAllocator& platform_allocator) ...@@ -148,6 +148,7 @@ CagedHeap::CagedHeap(PageAllocator& platform_allocator)
void CagedHeap::NotifyLargePageCreated(LargePage* page) { void CagedHeap::NotifyLargePageCreated(LargePage* page) {
DCHECK(page); DCHECK(page);
v8::base::MutexGuard guard(&large_pages_mutex_);
auto result = large_pages_.insert(page); auto result = large_pages_.insert(page);
USE(result); USE(result);
DCHECK(result.second); DCHECK(result.second);
...@@ -155,6 +156,7 @@ void CagedHeap::NotifyLargePageCreated(LargePage* page) { ...@@ -155,6 +156,7 @@ void CagedHeap::NotifyLargePageCreated(LargePage* page) {
void CagedHeap::NotifyLargePageDestroyed(LargePage* page) { void CagedHeap::NotifyLargePageDestroyed(LargePage* page) {
DCHECK(page); DCHECK(page);
v8::base::MutexGuard guard(&large_pages_mutex_);
auto size = large_pages_.erase(page); auto size = large_pages_.erase(page);
USE(size); USE(size);
DCHECK_EQ(1u, size); DCHECK_EQ(1u, size);
...@@ -170,6 +172,7 @@ BasePage& CagedHeap::LookupPageFromInnerPointer(void* ptr) const { ...@@ -170,6 +172,7 @@ BasePage& CagedHeap::LookupPageFromInnerPointer(void* ptr) const {
} }
LargePage& CagedHeap::LookupLargePageFromInnerPointer(void* ptr) const { LargePage& CagedHeap::LookupLargePageFromInnerPointer(void* ptr) const {
v8::base::MutexGuard guard(&large_pages_mutex_);
auto it = large_pages_.upper_bound(static_cast<LargePage*>(ptr)); auto it = large_pages_.upper_bound(static_cast<LargePage*>(ptr));
DCHECK_NE(large_pages_.begin(), it); DCHECK_NE(large_pages_.begin(), it);
auto* page = *std::next(it, -1); auto* page = *std::next(it, -1);
...@@ -179,6 +182,7 @@ LargePage& CagedHeap::LookupLargePageFromInnerPointer(void* ptr) const { ...@@ -179,6 +182,7 @@ LargePage& CagedHeap::LookupLargePageFromInnerPointer(void* ptr) const {
} }
void CagedHeap::ResetForTesting() { void CagedHeap::ResetForTesting() {
v8::base::MutexGuard guard(&large_pages_mutex_);
// Clear the large pages to support tests within the same process. // Clear the large pages to support tests within the same process.
large_pages_.clear(); large_pages_.clear();
} }
......
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "include/cppgc/platform.h" #include "include/cppgc/platform.h"
#include "src/base/bounded-page-allocator.h" #include "src/base/bounded-page-allocator.h"
#include "src/base/lazy-instance.h" #include "src/base/lazy-instance.h"
#include "src/base/platform/mutex.h"
#include "src/heap/cppgc/globals.h" #include "src/heap/cppgc/globals.h"
#include "src/heap/cppgc/virtual-memory.h" #include "src/heap/cppgc/virtual-memory.h"
...@@ -97,9 +98,15 @@ class V8_EXPORT_PRIVATE CagedHeap final { ...@@ -97,9 +98,15 @@ class V8_EXPORT_PRIVATE CagedHeap final {
static CagedHeap* instance_; static CagedHeap* instance_;
const VirtualMemory reserved_area_; const VirtualMemory reserved_area_;
// BoundedPageAllocator is thread-safe, no need to use external
// synchronization.
std::unique_ptr<AllocatorType> normal_page_bounded_allocator_; std::unique_ptr<AllocatorType> normal_page_bounded_allocator_;
std::unique_ptr<AllocatorType> large_page_bounded_allocator_; std::unique_ptr<AllocatorType> large_page_bounded_allocator_;
std::set<LargePage*> large_pages_; std::set<LargePage*> large_pages_;
// TODO(chromium:1325007): Since writes are rare, consider using read-write
// lock to speed up reading.
mutable v8::base::Mutex large_pages_mutex_;
}; };
} // namespace internal } // namespace internal
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment