AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

cppgc: Add targeted CHECK for diagnosing Peristent issue

Browse Source 
The added CHECK aims at finding problems where Peristent is used off
the owning thread.

Bug: chromium:1253650, chromium:1243257
Change-Id: Ia0cbc6005aba38c0d98197ed18c3b40dd2dc33fd
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3306972
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Anton Bikineev <bikineev@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78137}
This commit is contained in:
Michael Lippautz 2021-11-29 20:02:46 +01:00 committed by V8 LUCI CQ
parent fedeb2fc42
commit 4997ce58dd
3 changed files with 33 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
include/cppgc/internal/persistent-node.h
View File

@ -141,18 +141,18 @@ class V8_EXPORT PersistentRegion final : public PersistentRegionBase {
PersistentRegion& operator=(const PersistentRegion&) = delete;
V8_INLINE PersistentNode* AllocateNode(void* owner, TraceCallback trace) {
CPPGC_DCHECK(IsCreationThread());
CPPGC_CHECK(IsCreationThread());
return PersistentRegionBase::AllocateNode(owner, trace);
}
V8_INLINE void FreeNode(PersistentNode* node) {
CPPGC_DCHECK(IsCreationThread());
CPPGC_CHECK(IsCreationThread());
PersistentRegionBase::FreeNode(node);
}
private:
bool IsCreationThread();
private:
int creation_thread_id_;
};

16
include/cppgc/persistent.h
View File

@ -190,7 +190,13 @@ class BasicPersistent final : public PersistentBase,
// based on their actual types.
V8_CLANG_NO_SANITIZE("cfi-unrelated-cast") T* Get() const {
// TODO(chromium:1253650): Temporary CHECK to diagnose issues.
CPPGC_CHECK(IsValid() == !!GetNode());
if (IsValid()) {
CPPGC_CHECK(
WeaknessPolicy::GetPersistentRegion(GetValue()).IsCreationThread());
CPPGC_CHECK(GetNode() != nullptr);
} else {
CPPGC_CHECK(GetNode() == nullptr);
}
// The const_cast below removes the constness from PersistentBase storage.
// The following static_cast re-adds any constness if specified through the
@ -200,7 +206,13 @@ class BasicPersistent final : public PersistentBase,
void Clear() {
// TODO(chromium:1253650): Temporary CHECK to diagnose issues.
CPPGC_CHECK(IsValid() == !!GetNode());
if (IsValid()) {
CPPGC_CHECK(
WeaknessPolicy::GetPersistentRegion(GetValue()).IsCreationThread());
CPPGC_CHECK(GetNode() != nullptr);
} else {
CPPGC_CHECK(GetNode() == nullptr);
}
// Simplified version of `Assign()` to allow calling without a complete type
// `T`.
if (IsValid()) {

18
src/heap/cppgc/persistent-node.cc
View File

@ -103,14 +103,28 @@ void PersistentRegionBase::Trace(Visitor* visitor) {
nodes_.end());
}
namespace {
thread_local int thread_id = 0;
std::atomic<int> next_thread_id{1};
int GetCurrentThreadId() {
if (thread_id == 0) {
thread_id = next_thread_id.fetch_add(1);
}
return thread_id;
}
} // namespace
PersistentRegion::PersistentRegion(const FatalOutOfMemoryHandler& oom_handler)
: PersistentRegionBase(oom_handler),
creation_thread_id_(v8::base::OS::GetCurrentThreadId()) {
creation_thread_id_(GetCurrentThreadId()) {
USE(creation_thread_id_);
}
bool PersistentRegion::IsCreationThread() {
return creation_thread_id_ == v8::base::OS::GetCurrentThreadId();
return creation_thread_id_ == GetCurrentThreadId();
}
PersistentRegionLock::PersistentRegionLock() {