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

Revert "cppgc: Add targeted CHECK for diagnosing Peristent issue"

Browse Source 
This reverts commit 4997ce58dd.

Reason for revert: CHECK triggers on content_browsertests, blocking V8 roll
https://cr-buildbucket.appspot.com/build/8829191878491185313

Original change's description:
> cppgc: Add targeted CHECK for diagnosing Peristent issue
>
> 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}

Bug: chromium:1253650, chromium:1243257
Change-Id: I6b5c3d3ac0a01e1e3de31a10d5903ea26cf5ae9a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3308373
Auto-Submit: Adam Klein <adamk@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#78142}
This commit is contained in:
Adam Klein 2021-11-30 04:07:54 +00:00 committed by V8 LUCI CQ
parent f9cb3fe60c
commit 5d787082b1
3 changed files with 7 additions and 33 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_CHECK(IsCreationThread());
CPPGC_DCHECK(IsCreationThread());
return PersistentRegionBase::AllocateNode(owner, trace);
}
V8_INLINE void FreeNode(PersistentNode* node) {
CPPGC_CHECK(IsCreationThread());
CPPGC_DCHECK(IsCreationThread());
PersistentRegionBase::FreeNode(node);
}
private:
bool IsCreationThread();
private:
int creation_thread_id_;
};

16
include/cppgc/persistent.h
View File

@ -190,13 +190,7 @@ 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.
if (IsValid()) {
CPPGC_CHECK(
WeaknessPolicy::GetPersistentRegion(GetValue()).IsCreationThread());
CPPGC_CHECK(GetNode() != nullptr);
} else {
CPPGC_CHECK(GetNode() == nullptr);
}
CPPGC_CHECK(IsValid() == !!GetNode());
// The const_cast below removes the constness from PersistentBase storage.
// The following static_cast re-adds any constness if specified through the
@ -206,13 +200,7 @@ class BasicPersistent final : public PersistentBase,
void Clear() {
// TODO(chromium:1253650): Temporary CHECK to diagnose issues.
if (IsValid()) {
CPPGC_CHECK(
WeaknessPolicy::GetPersistentRegion(GetValue()).IsCreationThread());
CPPGC_CHECK(GetNode() != nullptr);
} else {
CPPGC_CHECK(GetNode() == nullptr);
}
CPPGC_CHECK(IsValid() == !!GetNode());
// Simplified version of `Assign()` to allow calling without a complete type
// `T`.
if (IsValid()) {

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

@ -103,28 +103,14 @@ 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_(GetCurrentThreadId()) {
creation_thread_id_(v8::base::OS::GetCurrentThreadId()) {
USE(creation_thread_id_);
}
bool PersistentRegion::IsCreationThread() {
return creation_thread_id_ == GetCurrentThreadId();
return creation_thread_id_ == v8::base::OS::GetCurrentThreadId();
}
PersistentRegionLock::PersistentRegionLock() {