Check if Member owner thread matches pointer thread and current thread

third_party/WebKit/Source/platform/heap/Member.h

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef Member_h
 #define Member_h
 
 #include "wtf/Allocator.h"
 #include "wtf/HashFunctions.h"
 #include "wtf/HashTraits.h"
 
 namespace blink {
 
 template<typename T> class Persistent;
 
+enum TracenessMemberConfiguration {

[haraken, 2016/08/16 13:27:13]

Shall we use an enum class?

enum class TracedMemberConfiguration {
  Traced,
  Untraced,
};

[keishi, 2016/08/17 11:18:29]

Done.

+    TracedMemberConfiguration,
+    UntracedMemberConfiguration
+};
+
 // Members are used in classes to contain strong pointers to other oilpan heap
 // allocated objects.
 // All Member fields of a class must be traced in the class' trace method.
 // During the mark phase of the GC all live objects are marked as live and
 // all Member fields of a live object will be traced marked as live as well.

[haraken, 2016/08/16 13:27:13]

Remove this comment.

[keishi, 2016/08/17 11:18:28]

Done.

-template<typename T>
-class Member {
+template<typename T, TracenessMemberConfiguration tracenessConfiguration = TracedMemberConfiguration>
+class MemberBase {
     DISALLOW_NEW_EXCEPT_PLACEMENT_NEW();
 public:
-    Member() : m_raw(nullptr)
+    MemberBase() : m_raw(nullptr)
     {
+        saveCreationThreadHeap();
     }
 
-    Member(std::nullptr_t) : m_raw(nullptr)
+    MemberBase(std::nullptr_t) : m_raw(nullptr)
     {
+        saveCreationThreadHeap();
     }
 
-    Member(T* raw) : m_raw(raw)
+    MemberBase(T* raw) : m_raw(raw)
     {
+        saveCreationThreadHeap();
         checkPointer();
     }
 
-    explicit Member(T& raw) : m_raw(&raw)
+    explicit MemberBase(T& raw) : m_raw(&raw)
     {
+        saveCreationThreadHeap();
         checkPointer();
     }
 
-    Member(WTF::HashTableDeletedValueType) : m_raw(reinterpret_cast<T*>(-1))
+    MemberBase(WTF::HashTableDeletedValueType) : m_raw(reinterpret_cast<T*>(-1))
     {
+        saveCreationThreadHeap();
     }
 
     bool isHashTableDeletedValue() const { return m_raw == reinterpret_cast<T*>(-1); }
 
-    Member(const Member& other) : m_raw(other)
+    MemberBase(const MemberBase& other) : m_raw(other)
     {
+        saveCreationThreadHeap();
         checkPointer();
     }
 
     template<typename U>
-    Member(const Persistent<U>& other)
+    MemberBase(const Persistent<U>& other)
     {
+        saveCreationThreadHeap();
         m_raw = other;
         checkPointer();
     }
 
     template<typename U>
-    Member(const Member<U>& other) : m_raw(other)
+    MemberBase(const MemberBase<U>& other) : m_raw(other)
     {
+        saveCreationThreadHeap();
         checkPointer();
     }
 
     T* release()
     {
         T* result = m_raw;
         m_raw = nullptr;
         return result;
     }
 
     explicit operator bool() const { return m_raw; }
 
     operator T*() const { return m_raw; }
 
     T* operator->() const { return m_raw; }
     T& operator*() const { return *m_raw; }
 
     template<typename U>
-    Member& operator=(const Persistent<U>& other)
+    MemberBase& operator=(const Persistent<U>& other)
     {
         m_raw = other;
         checkPointer();
         return *this;
     }
 
     template<typename U>
-    Member& operator=(const Member<U>& other)
+    MemberBase& operator=(const MemberBase<U>& other)
     {
         m_raw = other;
         checkPointer();
         return *this;
     }
 
     template<typename U>
-    Member& operator=(U* other)
+    MemberBase& operator=(U* other)
     {
         m_raw = other;
         checkPointer();
         return *this;
     }
 
-    Member& operator=(std::nullptr_t)
+    MemberBase& operator=(std::nullptr_t)
     {
         m_raw = nullptr;
         return *this;
     }
 
-    void swap(Member<T>& other)
+    void swap(MemberBase<T>& other)
     {
         std::swap(m_raw, other.m_raw);
         checkPointer();
     }
 
     T* get() const { return m_raw; }
 
     void clear() { m_raw = nullptr; }
 
 
 protected:
     void checkPointer()
     {
-#if ENABLE(ASSERT) && defined(ADDRESS_SANITIZER)
+#if DCHECK_IS_ON()
         if (!m_raw)
             return;
         // HashTable can store a special value (which is not aligned to the
         // allocation granularity) to Member<> to represent a deleted entry.
         // Thus we treat a pointer that is not aligned to the granularity
         // as a valid pointer.
         if (reinterpret_cast<intptr_t>(m_raw) % allocationGranularity)
             return;
 
+        ThreadState* current = ThreadState::current();
+        if (tracenessConfiguration != UntracedMemberConfiguration && current) {

[keishi, 2016/08/16 13:05:49]

Ignoring UntracedMember because in LayoutTests, GraphicsLayer::TakeDebugInfo
gets called on the compositor thread, which calls LayoutObject::debugNode which
creates a UntracedMember<Node>

[haraken, 2016/08/16 13:27:13]

'tracenessConfiguration != UntracedMemberConfiguration' makes sense, but do we
need to check 'current'?

If ThreadState::current() is nullptr, Member/WeakMember should not be used
whereas UntracedMember may be allowed.

[keishi, 2016/08/17 11:18:29]

Done. 

+            if (m_creationThreadHeap) {

[haraken, 2016/08/16 13:27:13]

How is it possible that m_creationThreadHeap is nullptr when we reach here?

[keishi, 2016/08/17 11:18:28]

I think heap collections use memset to initialize the Member and don't call the
constructor.

[haraken, 2016/08/17 11:40:41]

OK, let's add a comment about it.

[keishi, 2016/08/17 12:23:20]

Done.

+                DCHECK_EQ(&ThreadState::fromObject(m_raw)->heap(), m_creationThreadHeap);
+                DCHECK_EQ(&current->heap(), m_creationThreadHeap);
+            } else {
+                DCHECK(&ThreadState::fromObject(m_raw)->heap() == &current->heap());

[haraken, 2016/08/16 13:27:13]

Use DCHECK_EQ.

[keishi, 2016/08/17 11:18:29]

Done.

+            }
+        }
+
+#if defined(ADDRESS_SANITIZER)
         // TODO(haraken): What we really want to check here is that the pointer
         // is a traceable object. In other words, the pointer is either of:
         //
         //   (a) a pointer to the head of an on-heap object.
         //   (b) a pointer to the head of an on-heap mixin object.
         //
         // We can check it by calling ThreadHeap::isHeapObjectAlive(m_raw),
         // but we cannot call it here because it requires to include T.h.
         // So we currently only try to implement the check for (a), but do
         // not insist that T's definition is in scope.
         if (IsFullyDefined<T>::value && !IsGarbageCollectedMixin<T>::value)
             ASSERT(HeapObjectHeader::fromPayload(m_raw)->checkHeader());
 #endif
+#endif
+    }
+
+    void saveCreationThreadHeap()

[keishi, 2016/08/16 13:05:50]

|this| may be on the stack, and if it is we can't tell it apart so I record
m_creationThreadHeap.

+    {
+#if DCHECK_IS_ON()
+        if (ThreadState::current() && tracenessConfiguration != UntracedMemberConfiguration)

[haraken, 2016/08/16 13:27:13]

Do we really need to check ThreadState::current()?

As far as I understand, if ThreadState::current() is nullptr, Member/WeakMember
should not be created. UntracedMember may be allowed though.

[keishi, 2016/08/17 11:18:28]

Done.

+            m_creationThreadHeap = &ThreadState::current()->heap();
+        else
+            m_creationThreadHeap = nullptr;
+#endif
     }
 
     T* m_raw;
+#if DCHECK_IS_ON()
+    const ThreadHeap* m_creationThreadHeap;
+#endif
 
     template<bool x, WTF::WeakHandlingFlag y, WTF::ShouldWeakPointersBeMarkedStrongly z, typename U, typename V> friend struct CollectionBackingTraceTrait;
     friend class Visitor;
 
 };
 
+// Members are used in classes to contain strong pointers to other oilpan heap
+// allocated objects.
+// All Member fields of a class must be traced in the class' trace method.
+// During the mark phase of the GC all live objects are marked as live and
+// all Member fields of a live object will be traced marked as live as well.
+template<typename T>
+class Member : public MemberBase<T, TracedMemberConfiguration> {
+    DISALLOW_NEW_EXCEPT_PLACEMENT_NEW();
+    typedef MemberBase<T, TracedMemberConfiguration> Parent;
+public:
+    Member() : Parent() { }
+    Member(std::nullptr_t) : Parent(nullptr) { }
+    Member(T* raw) : Parent(raw) { }
+    Member(T& raw) : Parent(raw) { }
+    Member(WTF::HashTableDeletedValueType x) : Parent(x) { }
+
+    Member(const Member& other) : Parent(other) { }
+    template<typename U>
+    Member(const Member<U>& other) : Parent(other) { }
+    template<typename U>
+    Member(const Persistent<U>& other) : Parent(other) { }
+
+    template<typename U>
+    Member& operator=(const Persistent<U>& other)
+    {
+        Parent::operator=(other);
+        return *this;
+    }
+
+    template<typename U>
+    Member& operator=(const Member<U>& other)
+    {
+        Parent::operator=(other);
+        return *this;
+    }
+
+    template<typename U>
+    Member& operator=(const WeakMember<U>& other)
+    {
+        Parent::operator=(other);
+        return *this;
+    }
+
+    template<typename U>
+    Member& operator=(U* other)
+    {
+        Parent::operator=(other);
+        return *this;
+    }
+
+    Member& operator=(std::nullptr_t)
+    {
+        Parent::operator=(nullptr);
+        return *this;
+    }
+
+protected:
+    template<bool x, WTF::WeakHandlingFlag y, WTF::ShouldWeakPointersBeMarkedStrongly z, typename U, typename V> friend struct CollectionBackingTraceTrait;
+    friend class Visitor;
+};
+
 // WeakMember is similar to Member in that it is used to point to other oilpan
 // heap allocated objects.
 // However instead of creating a strong pointer to the object, the WeakMember creates
 // a weak pointer, which does not keep the pointee alive. Hence if all pointers to
 // to a heap allocated object are weak the object will be garbage collected. At the
 // time of GC the weak pointers will automatically be set to null.
 template<typename T>
-class WeakMember : public Member<T> {
+class WeakMember : public MemberBase<T, TracedMemberConfiguration> {
+    typedef MemberBase<T, TracedMemberConfiguration> Parent;
 public:
-    WeakMember() : Member<T>() { }
+    WeakMember() : Parent() { }
 
-    WeakMember(std::nullptr_t) : Member<T>(nullptr) { }
+    WeakMember(std::nullptr_t) : Parent(nullptr) { }
 
-    WeakMember(T* raw) : Member<T>(raw) { }
+    WeakMember(T* raw) : Parent(raw) { }
 
-    WeakMember(WTF::HashTableDeletedValueType x) : Member<T>(x) { }
+    WeakMember(WTF::HashTableDeletedValueType x) : Parent(x) { }
 
     template<typename U>
-    WeakMember(const Persistent<U>& other) : Member<T>(other) { }
+    WeakMember(const Persistent<U>& other) : Parent(other) { }
 
     template<typename U>
-    WeakMember(const Member<U>& other) : Member<T>(other) { }
+    WeakMember(const Member<U>& other) : Parent(other) { }
 
     template<typename U>
     WeakMember& operator=(const Persistent<U>& other)
     {
         this->m_raw = other;
         this->checkPointer();
         return *this;
     }
 
     template<typename U>
@@ skipping 26 matching lines @@
 
 // UntracedMember is a pointer to an on-heap object that is not traced for some
 // reason. Please don't use this unless you understand what you're doing.
 // Basically, all pointers to on-heap objects must be stored in either of
 // Persistent, Member or WeakMember. It is not allowed to leave raw pointers to
 // on-heap objects. However, there can be scenarios where you have to use raw
 // pointers for some reason, and in that case you can use UntracedMember. Of
 // course, it must be guaranteed that the pointing on-heap object is kept alive
 // while the raw pointer is pointing to the object.
 template<typename T>
-class UntracedMember final : public Member<T> {
+class UntracedMember final : public MemberBase<T, UntracedMemberConfiguration> {
+    typedef MemberBase<T, UntracedMemberConfiguration> Parent;
 public:
-    UntracedMember() : Member<T>() { }
+    UntracedMember() : Parent() { }
 
-    UntracedMember(std::nullptr_t) : Member<T>(nullptr) { }
+    UntracedMember(std::nullptr_t) : Parent(nullptr) { }
 
-    UntracedMember(T* raw) : Member<T>(raw) { }
+    UntracedMember(T* raw) : Parent(raw) { }
 
     template<typename U>
-    UntracedMember(const Persistent<U>& other) : Member<T>(other) { }
+    UntracedMember(const Persistent<U>& other) : Parent(other) { }
 
     template<typename U>
-    UntracedMember(const Member<U>& other) : Member<T>(other) { }
+    UntracedMember(const Member<U>& other) : Parent(other) { }
 
-    UntracedMember(WTF::HashTableDeletedValueType x) : Member<T>(x) { }
+    UntracedMember(WTF::HashTableDeletedValueType x) : Parent(x) { }
 
     template<typename U>
     UntracedMember& operator=(const Persistent<U>& other)
     {
         this->m_raw = other;
         this->checkPointer();
         return *this;
     }
 
     template<typename U>
@@ skipping 65 matching lines @@
 
 template<typename T>
 struct IsTraceable<blink::WeakMember<T>> {
     STATIC_ONLY(IsTraceable);
     static const bool value = true;
 };
 
 } // namespace WTF
 
 #endif // Member_h