Restrict registered PersistentNodes to non-empty ones.

Source/platform/heap/Handle.h

 /*
  * Copyright (C) 2014 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 120 matching lines @@
             visitor->registerWeakCell(&m_raw);
         } else {
             visitor->mark(m_raw);
         }
     }
 
     RawPtr<T> release()
     {
         RawPtr<T> result = m_raw;
         m_raw = nullptr;
+        updatePersistentNode();
         return result;
     }
 
-    void clear() { m_raw = nullptr; }
+    void clear()
+    {
+        m_raw = nullptr;
+        updatePersistentNode();
+    }
     T& operator*() const { return *m_raw; }
     bool operator!() const { return !m_raw; }
     operator T*() const { return m_raw; }
     operator RawPtr<T>() const { return m_raw; }
     T* operator->() const { return *this; }
     T* get() const { return m_raw; }
 
     template<typename U>
     PersistentBase& operator=(U* other)
     {
         m_raw = other;
+        updatePersistentNode();
         checkPointer();
         recordBacktrace();
         return *this;
     }
 
     PersistentBase& operator=(std::nullptr_t)
     {
         m_raw = nullptr;
+        updatePersistentNode();
         return *this;
     }
 
     PersistentBase& operator=(const PersistentBase& other)
     {
         m_raw = other;
+        updatePersistentNode();
         checkPointer();
         recordBacktrace();
         return *this;
     }
 
     template<typename U>
     PersistentBase& operator=(const PersistentBase<U, weaknessConfiguration, crossThreadnessConfiguration>& other)
     {
         m_raw = other;
+        updatePersistentNode();
         checkPointer();
         recordBacktrace();
         return *this;
     }
 
     template<typename U>
     PersistentBase& operator=(const Member<U>& other)
     {
         m_raw = other;
+        updatePersistentNode();
         checkPointer();
         recordBacktrace();
         return *this;
     }
 
     template<typename U>
     PersistentBase& operator=(const RawPtr<U>& other)
     {
         m_raw = other;
+        updatePersistentNode();
         checkPointer();
         recordBacktrace();
         return *this;
     }
 
 
 private:
     NO_LAZY_SWEEP_SANITIZE_ADDRESS
+    void updatePersistentNode()

[haraken, 2015/09/13 12:36:56]

Maybe I'd prefer renaming this to assign() and do 'm_raw = ...' in assign().

void assign(T* value) {
  m_raw = value;
  if (m_raw)
    ...;
}

Also you can move checkPointer() into assign().

[sof, 2015/09/14 06:34:04]

Switched to that scheme; I think it is ok to leave it to the compiler whether or
not to inline.

+    {
+        if (m_raw) {
+            if (!m_persistentNode)
+                initialize();
+            return;
+        }
+        if (m_persistentNode && crossThreadnessConfiguration != CrossThreadPersistentConfiguration)
+            uninitialize();

[haraken, 2015/09/13 12:36:56]

Will this remove the restriction that a (non-cross-thread) persistent handle
must be destructed before the thread shuts down? After this CL, the thread can
shut down if the persistent handle is cleared (doesn't need to be destructed).

[sof, 2015/09/14 06:34:04]

It removes that restriction, as detailed on
https://codereview.chromium.org/1338573003/#msg17

+    }
+
+    NO_LAZY_SWEEP_SANITIZE_ADDRESS
     void initialize()
     {
+        if (m_persistentNode || !m_raw)

[haraken, 2015/09/13 12:36:56]

Is it possible that m_persistentNode is non-null here?

[sof, 2015/09/14 06:34:04]

It no longer is, thanks - replaced with an assert.

+            return;
+
         TraceCallback traceCallback = TraceMethodDelegate<PersistentBase<T, weaknessConfiguration, crossThreadnessConfiguration>, &PersistentBase<T, weaknessConfiguration, crossThreadnessConfiguration>::trace>::trampoline;
         if (crossThreadnessConfiguration == CrossThreadPersistentConfiguration) {
             m_persistentNode = ThreadState::crossThreadPersistentRegion().allocatePersistentNode(this, traceCallback);
         } else {
             ThreadState* state = ThreadStateFor<ThreadingTrait<T>::Affinity>::state();
             ASSERT(state->checkThread());
             m_persistentNode = state->persistentRegion()->allocatePersistentNode(this, traceCallback);
 #if ENABLE(ASSERT)
             m_state = state;
 #endif
         }
     }
 
     void uninitialize()
     {
+        if (!m_persistentNode)
+            return;
+
         if (crossThreadnessConfiguration == CrossThreadPersistentConfiguration) {
             ThreadState::crossThreadPersistentRegion().freePersistentNode(m_persistentNode);
         } else {
             ThreadState* state = ThreadStateFor<ThreadingTrait<T>::Affinity>::state();
             ASSERT(state->checkThread());
             // Persistent handle must be created and destructed in the same thread.
             ASSERT(m_state == state);
             state->persistentRegion()->freePersistentNode(m_persistentNode);
         }
+        m_persistentNode = nullptr;
     }
 
     void checkPointer()
     {
 #if ENABLE(ASSERT)
         if (!m_raw)
             return;
 
         // Heap::isHeapObjectAlive(m_raw) checks that m_raw is a traceable
         // object. In other words, it checks that the pointer is either of:
@@ skipping 13 matching lines @@
         if (m_raw)
             m_tracingName = Heap::createBacktraceString();
     }
 
     String m_tracingName;
 #else
     inline void recordBacktrace() const { }
 #endif
     // m_raw is accessed most, so put it at the first field.
     T* m_raw;
-    PersistentNode* m_persistentNode;
+    PersistentNode* m_persistentNode = nullptr;
 #if ENABLE(ASSERT)
-    ThreadState* m_state;
+    ThreadState* m_state = nullptr;
 #endif
 };
 
 // Persistent is a way to create a strong pointer from an off-heap object
 // to another on-heap object. As long as the Persistent handle is alive
 // the GC will keep the object pointed to alive. The Persistent handle is
 // always a GC root from the point of view of the GC.
 //
 // We have to construct and destruct Persistent in the same thread.
 template<typename T>
@@ skipping 932 matching lines @@
     // TODO(sof): extend WTF::FunctionWrapper call overloading to also handle (CrossThread)WeakPersistent.
     static T* unwrap(const StorageType& value) { return value.get(); }
 };
 
 template<typename T>
 PassRefPtr<T> adoptRef(blink::RefCountedGarbageCollected<T>*) = delete;
 
 } // namespace WTF
 
 #endif