Commit f6bc6b6d authored by Omer Katz's avatar Omer Katz Committed by Commit Bot

cppgc: Thread safe JSMember

Make all writes to JSMember.val_ atomic and atomically check for
emptiness in Trace.

Bug: chromium:1056170
Change-Id: Ia7034b9318df081aa61c9b6664903dd4f73402a5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2431569Reviewed-by: 's avatarMichael Lippautz <mlippautz@chromium.org>
Commit-Queue: Omer Katz <omerkatz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70309}
parent ff61743f
...@@ -14,6 +14,7 @@ namespace v8 { ...@@ -14,6 +14,7 @@ namespace v8 {
class Isolate; class Isolate;
template <typename T> template <typename T>
class JSMember; class JSMember;
class JSVisitor;
namespace internal { namespace internal {
...@@ -49,11 +50,21 @@ class V8_EXPORT JSMemberBase { ...@@ -49,11 +50,21 @@ class V8_EXPORT JSMemberBase {
inline JSMemberBase& CopyImpl(const JSMemberBase& other); inline JSMemberBase& CopyImpl(const JSMemberBase& other);
inline JSMemberBase& MoveImpl(JSMemberBase&& other); inline JSMemberBase& MoveImpl(JSMemberBase&& other);
void SetSlotThreadSafe(void* value) {
reinterpret_cast<std::atomic<void*>*>(&val_)->store(
value, std::memory_order_relaxed);
}
bool IsEmptyThreadSafe() const {
return reinterpret_cast<std::atomic<const void*> const*>(&val_)->load(
std::memory_order_relaxed) == nullptr;
}
// val_ points to a GlobalHandles node. // val_ points to a GlobalHandles node.
internal::Address* val_ = nullptr; internal::Address* val_ = nullptr;
template <typename T> template <typename T>
friend class v8::JSMember; friend class v8::JSMember;
friend class v8::JSVisitor;
friend class v8::internal::JSMemberBaseExtractor; friend class v8::internal::JSMemberBaseExtractor;
}; };
...@@ -79,7 +90,7 @@ JSMemberBase& JSMemberBase::MoveImpl(JSMemberBase&& other) { ...@@ -79,7 +90,7 @@ JSMemberBase& JSMemberBase::MoveImpl(JSMemberBase&& other) {
void JSMemberBase::Reset() { void JSMemberBase::Reset() {
if (IsEmpty()) return; if (IsEmpty()) return;
Delete(val_); Delete(val_);
val_ = nullptr; SetSlotThreadSafe(nullptr);
} }
} // namespace internal } // namespace internal
...@@ -144,7 +155,8 @@ class V8_EXPORT JSMember : public internal::JSMemberBase { ...@@ -144,7 +155,8 @@ class V8_EXPORT JSMember : public internal::JSMemberBase {
typename = std::enable_if_t<std::is_base_of<T, U>::value>> typename = std::enable_if_t<std::is_base_of<T, U>::value>>
void Set(v8::Isolate* isolate, Local<U> that) { void Set(v8::Isolate* isolate, Local<U> that) {
Reset(); Reset();
val_ = New(isolate, reinterpret_cast<internal::Address*>(*that), &val_); SetSlotThreadSafe(
New(isolate, reinterpret_cast<internal::Address*>(*that), &val_));
} }
}; };
...@@ -200,7 +212,7 @@ class JSVisitor : public cppgc::Visitor { ...@@ -200,7 +212,7 @@ class JSVisitor : public cppgc::Visitor {
template <typename T> template <typename T>
void Trace(const JSMember<T>& ref) { void Trace(const JSMember<T>& ref) {
if (ref.IsEmpty()) return; if (ref.IsEmptyThreadSafe()) return;
Visit(ref); Visit(ref);
} }
......
...@@ -1073,7 +1073,7 @@ void GlobalHandles::MoveTracedGlobal(Address** from, Address** to) { ...@@ -1073,7 +1073,7 @@ void GlobalHandles::MoveTracedGlobal(Address** from, Address** to) {
} }
} }
DestroyTraced(*from); DestroyTraced(*from);
*from = nullptr; SetSlotThreadSafe(from, nullptr);
} else { } else {
// Pure heap move. // Pure heap move.
DestroyTraced(*to); DestroyTraced(*to);
...@@ -1086,7 +1086,7 @@ void GlobalHandles::MoveTracedGlobal(Address** from, Address** to) { ...@@ -1086,7 +1086,7 @@ void GlobalHandles::MoveTracedGlobal(Address** from, Address** to) {
if (to_node->has_destructor()) { if (to_node->has_destructor()) {
to_node->set_parameter(to); to_node->set_parameter(to);
} }
*from = nullptr; SetSlotThreadSafe(from, nullptr);
} }
TracedNode::Verify(global_handles, to); TracedNode::Verify(global_handles, to);
} }
......
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