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

Revert "cppgc: Fix CTP for destruction"

Browse Source 
This reverts commit 5a6c7dee4e.

Reason for revert: Speculative: Lots of Chrome crashes:
https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Blink%20Linux/13353/overview

Original change's description:
> cppgc: Fix CTP for destruction
>
> This avoids a benign race in setting the raw pointer inside CTP
> destructor by not emitting the write at all. The handle is destructed
> which means that we only need to destroy any backing node but may
> leave the handle untouched.
>
> Drive-by:
> - Add a few more docs.
> - Make Clear() thread-safe.
> - Make assignment of a sentinel pointer thread-safe.
> - Make assignment of a nullptr thread-safe.
>
> Bug: chromium:1242795
> Change-Id: I0d9dafa31c298053e87ba1eb75f99fa6e33fa10b
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3114134
> Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
> Reviewed-by: Anton Bikineev <bikineev@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#76455}

Bug: chromium:1242795
Change-Id: Ia96d66f4908894091a4e498116d9568bd7b0e0a3
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3114058
Auto-Submit: Michael Achenbach <machenbach@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Owners-Override: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#76482}
This commit is contained in:
Michael Achenbach 2021-08-25 07:58:21 +00:00 committed by V8 LUCI CQ
parent e821cc7a50
commit 8ab11efbd5
1 changed files with 19 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
include/cppgc/cross-thread-persistent.h
View File

@ -75,27 +75,7 @@ class BasicCrossThreadPersistent final : public CrossThreadPersistentBase,
using typename WeaknessPolicy::IsStrongPersistent;
using PointeeType = T;
~BasicCrossThreadPersistent() {
// Simplified version of `Assign()` to allow calling without a complete type
// `T`. Also performs a thread-safe check for a handle that is not valid.
// This implements fast path for destroying empty/sentinel handles.
if (GetNodeSafe()) {
PersistentRegionLock guard;
const void* old_value = GetValue();
// The fast path check (GetNodeSafe()) does not acquire the lock. Recheck
// validity while holding the lock to ensure the reference has not been
// cleared.
if (IsValid(old_value)) {
CrossThreadPersistentRegion& region =
this->GetPersistentRegion(old_value);
region.FreeNode(GetNode());
SetNode(nullptr);
} else {
CPPGC_DCHECK(!GetNode());
}
}
// No need to call SetValue() as the handle is not used anymore.
}
~BasicCrossThreadPersistent() { Clear(); }
BasicCrossThreadPersistent(
const SourceLocation& loc = SourceLocation::Current())
@ -212,11 +192,6 @@ class BasicCrossThreadPersistent final : public CrossThreadPersistentBase,
return *this;
}
/**
* Assigns a raw pointer.
*
* Note: **Not thread-safe.**
*/
BasicCrossThreadPersistent& operator=(T* other) {
Assign(other);
return *this;
@ -233,24 +208,13 @@ class BasicCrossThreadPersistent final : public CrossThreadPersistentBase,
return operator=(member.Get());
}
/**
* Assigns a nullptr.
*
* \returns the handle.
*/
BasicCrossThreadPersistent& operator=(std::nullptr_t) {
Clear();
return *this;
}
/**
* Assigns the sentinel pointer.
*
* \returns the handle.
*/
BasicCrossThreadPersistent& operator=(SentinelPointer s) {
PersistentRegionLock guard;
AssignUnsafe(s);
Assign(s);
return *this;
}
@ -272,8 +236,23 @@ class BasicCrossThreadPersistent final : public CrossThreadPersistentBase,
* Clears the stored object.
*/
void Clear() {
PersistentRegionLock guard;
AssignUnsafe(nullptr);
// Simplified version of `Assign()` to allow calling without a complete type
// `T`. Also performs a thread-safe check for a handle that is not valid.
if (GetNodeSafe()) {
PersistentRegionLock guard;
const void* old_value = GetValue();
// The fast path check (IsValid()) does not acquire the lock. Reload
// the value to ensure the reference has not been cleared.
if (IsValid(old_value)) {
CrossThreadPersistentRegion& region =
this->GetPersistentRegion(old_value);
region.FreeNode(GetNode());
SetNode(nullptr);
} else {
CPPGC_DCHECK(!GetNode());
}
}
SetValue(nullptr);
}
/**