Commit ce336fdb authored by Michael Lippautz's avatar Michael Lippautz Committed by Commit Bot

cppgc: Fix {Weak}CrossThreadPersistent destruction

Bug: chromium:1056170
Change-Id: I89dd887a75a475f998d950e86f35c7fe2af5d67f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2743887Reviewed-by: 's avatarOmer Katz <omerkatz@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73307}
parent 0defc528
...@@ -192,10 +192,17 @@ class BasicCrossThreadPersistent final : public PersistentBase, ...@@ -192,10 +192,17 @@ class BasicCrossThreadPersistent final : public PersistentBase,
const void* old_value = GetValue(); const void* old_value = GetValue();
if (IsValid(old_value)) { if (IsValid(old_value)) {
PersistentRegionLock guard; PersistentRegionLock guard;
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 = CrossThreadPersistentRegion& region =
this->GetPersistentRegion(old_value); this->GetPersistentRegion(old_value);
region.FreeNode(GetNode()); region.FreeNode(GetNode());
SetNode(nullptr); SetNode(nullptr);
} else {
CPPGC_DCHECK(!GetNode());
}
} }
SetValue(nullptr); SetValue(nullptr);
} }
...@@ -277,6 +284,10 @@ class BasicCrossThreadPersistent final : public PersistentBase, ...@@ -277,6 +284,10 @@ class BasicCrossThreadPersistent final : public PersistentBase,
const void* old_value = GetValue(); const void* old_value = GetValue();
if (IsValid(old_value)) { if (IsValid(old_value)) {
PersistentRegionLock guard; PersistentRegionLock guard;
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 = CrossThreadPersistentRegion& region =
this->GetPersistentRegion(old_value); this->GetPersistentRegion(old_value);
if (IsValid(ptr) && (&region == &this->GetPersistentRegion(ptr))) { if (IsValid(ptr) && (&region == &this->GetPersistentRegion(ptr))) {
...@@ -286,6 +297,9 @@ class BasicCrossThreadPersistent final : public PersistentBase, ...@@ -286,6 +297,9 @@ class BasicCrossThreadPersistent final : public PersistentBase,
} }
region.FreeNode(GetNode()); region.FreeNode(GetNode());
SetNode(nullptr); SetNode(nullptr);
} else {
CPPGC_DCHECK(!GetNode());
}
} }
SetValue(ptr); SetValue(ptr);
if (!IsValid(ptr)) return; if (!IsValid(ptr)) return;
......
...@@ -99,6 +99,7 @@ class V8_EXPORT PersistentRegion final { ...@@ -99,6 +99,7 @@ class V8_EXPORT PersistentRegion final {
} }
void FreeNode(PersistentNode* node) { void FreeNode(PersistentNode* node) {
CPPGC_DCHECK(node);
CPPGC_DCHECK(node->IsUsed()); CPPGC_DCHECK(node->IsUsed());
node->InitializeAsFreeNode(free_list_head_); node->InitializeAsFreeNode(free_list_head_);
free_list_head_ = node; free_list_head_ = node;
......
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