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

heap: ConstructTraits for WeakMember

HashTable with Member had a data race that required adding an atomic
ctor for Member via ConstructTraits. It was not needed for WeakMember
because weak HashTables were traced only on mutator thread. Now these
tables can be traced concurrently so the same data race can occur for
WeakMember as well.

Bug: 986235
Change-Id: Ic54167be0dbcbb025b136ee023929125b1a33c55
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2352922
Commit-Queue: Omer Katz <omerkatz@chromium.org>
Commit-Queue: Anton Bikineev <bikineev@chromium.org>
Reviewed-by: default avatarAnton Bikineev <bikineev@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798119}
parent 1b331687
...@@ -15,7 +15,7 @@ ...@@ -15,7 +15,7 @@
namespace WTF { namespace WTF {
template <typename P, typename Traits, typename Allocator> template <typename P, typename Traits, typename Allocator>
class ConstructTraits; class MemberConstructTraits;
} // namespace WTF } // namespace WTF
namespace blink { namespace blink {
...@@ -343,7 +343,7 @@ class Member : public MemberBase<T, TracenessMemberConfiguration::kTraced> { ...@@ -343,7 +343,7 @@ class Member : public MemberBase<T, TracenessMemberConfiguration::kTraced> {
Member(AtomicCtorTag atomic, T& raw) : Parent(atomic, raw) {} Member(AtomicCtorTag atomic, T& raw) : Parent(atomic, raw) {}
template <typename P, typename Traits, typename Allocator> template <typename P, typename Traits, typename Allocator>
friend class WTF::ConstructTraits; friend class WTF::MemberConstructTraits;
}; };
// WeakMember is similar to Member in that it is used to point to other oilpan // WeakMember is similar to Member in that it is used to point to other oilpan
...@@ -394,6 +394,14 @@ class WeakMember : public MemberBase<T, TracenessMemberConfiguration::kTraced> { ...@@ -394,6 +394,14 @@ class WeakMember : public MemberBase<T, TracenessMemberConfiguration::kTraced> {
this->SetRaw(nullptr); this->SetRaw(nullptr);
return *this; return *this;
} }
private:
using typename Parent::AtomicCtorTag;
WeakMember(AtomicCtorTag atomic, T* raw) : Parent(atomic, raw) {}
WeakMember(AtomicCtorTag atomic, T& raw) : Parent(atomic, raw) {}
template <typename P, typename Traits, typename Allocator>
friend class WTF::MemberConstructTraits;
}; };
// UntracedMember is a pointer to an on-heap object that is not traced for some // UntracedMember is a pointer to an on-heap object that is not traced for some
...@@ -509,34 +517,29 @@ struct IsTraceable<blink::WeakMember<T>> { ...@@ -509,34 +517,29 @@ struct IsTraceable<blink::WeakMember<T>> {
}; };
template <typename T, typename Traits, typename Allocator> template <typename T, typename Traits, typename Allocator>
class ConstructTraits<blink::Member<T>, Traits, Allocator> { class MemberConstructTraits {
STATIC_ONLY(ConstructTraits); STATIC_ONLY(MemberConstructTraits);
public: public:
template <typename... Args> template <typename... Args>
static blink::Member<T>* Construct(void* location, Args&&... args) { static T* Construct(void* location, Args&&... args) {
return new (NotNull, location) return new (NotNull, location) T(std::forward<Args>(args)...);
blink::Member<T>(std::forward<Args>(args)...);
} }
static void NotifyNewElement(blink::Member<T>* element) { static void NotifyNewElement(T* element) { element->WriteBarrier(); }
element->WriteBarrier();
}
template <typename... Args> template <typename... Args>
static blink::Member<T>* ConstructAndNotifyElement(void* location, static T* ConstructAndNotifyElement(void* location, Args&&... args) {
Args&&... args) {
// ConstructAndNotifyElement updates an existing Member which might // ConstructAndNotifyElement updates an existing Member which might
// also be comncurrently traced while we update it. The regular ctors // also be comncurrently traced while we update it. The regular ctors
// for Member don't use an atomic write which can lead to data races. // for Member don't use an atomic write which can lead to data races.
blink::Member<T>* object = T* object = Construct(location, T::AtomicCtorTag::Atomic,
Construct(location, blink::Member<T>::AtomicCtorTag::Atomic, std::forward<Args>(args)...);
std::forward<Args>(args)...);
NotifyNewElement(object); NotifyNewElement(object);
return object; return object;
} }
static void NotifyNewElements(blink::Member<T>* array, size_t len) { static void NotifyNewElements(T* array, size_t len) {
while (len-- > 0) { while (len-- > 0) {
array->WriteBarrier(); array->WriteBarrier();
array++; array++;
...@@ -544,6 +547,14 @@ class ConstructTraits<blink::Member<T>, Traits, Allocator> { ...@@ -544,6 +547,14 @@ class ConstructTraits<blink::Member<T>, Traits, Allocator> {
} }
}; };
template <typename T, typename Traits, typename Allocator>
class ConstructTraits<blink::Member<T>, Traits, Allocator>
: public MemberConstructTraits<blink::Member<T>, Traits, Allocator> {};
template <typename T, typename Traits, typename Allocator>
class ConstructTraits<blink::WeakMember<T>, Traits, Allocator>
: public MemberConstructTraits<blink::WeakMember<T>, Traits, Allocator> {};
} // namespace WTF } // namespace WTF
#endif // THIRD_PARTY_BLINK_RENDERER_PLATFORM_HEAP_MEMBER_H_ #endif // THIRD_PARTY_BLINK_RENDERER_PLATFORM_HEAP_MEMBER_H_
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