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

heap: Additional atomic writes/reads in ListHashSet

This CL fixes a few atomic writes/reads in ListHashSet that were
previously missed:
1) Write when constructing a deleted value.
2) Write when assigning a new value (during translation).
3) Read to check if node was destructed.

These are very small and simple changes, thus I'm submitting them
as a single CL.

Bug: 986235
Change-Id: Ia1e89120506a802e7514633e42c929ddcbe33717
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2012387
Commit-Queue: Omer Katz <omerkatz@chromium.org>
Reviewed-by: default avatarMichael Lippautz <mlippautz@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#735865}
parent 11999f37
......@@ -70,6 +70,17 @@ struct ListHashSetNodeHashFunctions;
template <typename HashArg>
struct ListHashSetTranslator;
template <typename Value, typename Allocator>
struct ListHashSetTraits
: public HashTraits<ListHashSetNode<Value, Allocator>*> {
using Node = ListHashSetNode<Value, Allocator>;
static void ConstructDeletedValue(Node*& slot, bool) {
AsAtomicPtr(&slot)->store(reinterpret_cast<Node*>(-1),
std::memory_order_relaxed);
}
};
// Note that for a ListHashSet you cannot specify the HashTraits as a template
// argument. It uses the default hash traits for the ValueArg type.
template <typename ValueArg,
......@@ -85,7 +96,7 @@ class ListHashSet
USE_ALLOCATOR(ListHashSet, Allocator);
typedef ListHashSetNode<ValueArg, Allocator> Node;
typedef HashTraits<Node*> NodeTraits;
typedef ListHashSetTraits<ValueArg, Allocator> NodeTraits;
typedef ListHashSetNodeHashFunctions<HashArg> NodeHash;
typedef ListHashSetTranslator<HashArg> BaseTranslator;
......@@ -309,10 +320,7 @@ class ListHashSetNodeBasePointer {
private:
void SetSafe(NodeType* node) {
if (Allocator::kIsGarbageCollected)
AsAtomicPtr(&node_)->store(node, std::memory_order_relaxed);
else
node_ = node;
AsAtomicPtr(&node_)->store(node, std::memory_order_relaxed);
}
NodeType* GetSafe() const {
......@@ -522,7 +530,7 @@ class ListHashSetNode : public ListHashSetNodeBase<ValueArg, AllocatorArg> {
// node from the ListHashSet while an iterator is positioned at that
// node, so there should be no valid pointers from the stack to a
// destructed node.
if (WasAlreadyDestructed())
if (WasAlreadyDestructedSafe())
return;
NodeAllocator::TraceValue(visitor, this);
visitor->Trace(reinterpret_cast<ListHashSetNode*>(this->next_.GetSafe()));
......@@ -545,6 +553,12 @@ class ListHashSetNode : public ListHashSetNodeBase<ValueArg, AllocatorArg> {
template <typename HashArg>
friend struct ListHashSetNodeHashFunctions;
private:
bool WasAlreadyDestructedSafe() const {
DCHECK(NodeAllocator::kIsGarbageCollected);
return this->prev_.GetSafe() == UnlinkedNodePointer();
}
};
template <typename HashArg>
......@@ -818,7 +832,9 @@ struct ListHashSetTranslator {
}
template <typename T, typename U, typename V>
static void Translate(T*& location, U&& key, const V& allocator) {
location = new (const_cast<V*>(&allocator)) T(std::forward<U>(key));
AsAtomicPtr(&location)->store(new (const_cast<V*>(&allocator))
T(std::forward<U>(key)),
std::memory_order_relaxed);
}
};
......
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