Unify and generalize thread static persistent finalization.

third_party/WebKit/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 164 matching lines @@
         return *this;
     }
 
     template<typename U>
     PersistentBase& operator=(const Member<U>& other)
     {
         assign(other);
         return *this;
     }
 
-#if defined(LEAK_SANITIZER)
+    // Register the persistent node as a 'static reference',
+    // belonging to the current thread and a persistent that must
+    // be cleared when the ThreadState itself is cleared out and
+    // destructed.
+    //
+    // Static singletons arrange for this to happen, either to ensure
+    // clean LSan leak reports or to register a thread-local persistent
+    // needing to be cleared out before the thread is terminated.
     PersistentBase* registerAsStaticReference()
     {
         if (m_persistentNode) {
             ASSERT(ThreadState::current());
-            ThreadState::current()->registerStaticPersistentNode(m_persistentNode);
+            ThreadState::current()->registerStaticPersistentNode(m_persistentNode, nullptr);
             LEAK_SANITIZER_IGNORE_OBJECT(this);
         }
         return this;
     }
-#endif
 
 protected:
     T* atomicGet() { return reinterpret_cast<T*>(acquireLoad(reinterpret_cast<void* volatile*>(&m_raw))); }
 
 private:
     NO_LAZY_SWEEP_SANITIZE_ADDRESS
     void assign(T* ptr)
     {
         if (crossThreadnessConfiguration == CrossThreadPersistentConfiguration)
             releaseStore(reinterpret_cast<void* volatile*>(&m_raw), ptr);
@@ skipping 34 matching lines @@
         if (!m_persistentNode)
             return;
 
         if (crossThreadnessConfiguration == CrossThreadPersistentConfiguration) {
             ProcessHeap::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->getPersistentRegion()->freePersistentNode(m_persistentNode);
+            state->freePersistentNode(m_persistentNode);
         }
         m_persistentNode = nullptr;
     }
 
     void checkPointer()
     {
 #if ENABLE(ASSERT) && defined(ADDRESS_SANITIZER)
         if (!m_raw)
             return;
 
@@ skipping 62 matching lines @@
         Parent::operator=(other);
         return *this;
     }
 
     template<typename U>
     Persistent& operator=(const Member<U>& other)
     {
         Parent::operator=(other);
         return *this;
     }
-
-    // Requests that the thread state clear this handle when the thread shuts
-    // down. This is intended for use with ThreadSpecific<Persistent<T>>.
-    // It's important that the Persistent<T> exist until then, because this
-    // takes a raw pointer to that handle.
-    //
-    // Example:
-    //   Foo& sharedFoo()
-    //   {
-    //        DEFINE_THREAD_SAFE_STATIC_LOCAL(
-    //            ThreadSpecific<Persistent<Foo>>, threadSpecificFoo,
-    //            new ThreadSpecific<Persistent<Foo>>);
-    //        Persistent<Foo>& fooHandle = *threadSpecificFoo;
-    //        if (!fooHandle) {
-    //            fooHandle = new Foo;
-    //            fooHandle.clearOnThreadShutdown();
-    //        }
-    //        return *fooHandle;
-    //   }
-    void clearOnThreadShutdown()
-    {
-        void (*closure)(Persistent<T>*) = [](Persistent<T>* handle)
-        {
-            *handle = nullptr;
-        };
-        ThreadState::current()->registerThreadShutdownHook(WTF::bind(closure, this));
-    }
 };
 
 // WeakPersistent is a way to create a weak pointer from an off-heap object
 // to an on-heap object. The m_raw is automatically cleared when the pointee
 // gets collected.
 //
 // We have to construct and destruct WeakPersistent in the same thread.
 //
 // Note that collections of WeakPersistents are not supported. Use a persistent
 // collection of WeakMembers instead.
@@ skipping 178 matching lines @@
         uninitialize();
     }
 
     template<typename VisitorDispatcher>
     void trace(VisitorDispatcher visitor)
     {
         static_assert(sizeof(Collection), "Collection must be fully defined");
         visitor->trace(*static_cast<Collection*>(this));
     }
 
-#if defined(LEAK_SANITIZER)
+    // See PersistentBase::registerAsStaticReference() comment.
     PersistentHeapCollectionBase* registerAsStaticReference()
     {
         if (m_persistentNode) {
             ASSERT(ThreadState::current());
-            ThreadState::current()->registerStaticPersistentNode(m_persistentNode);
+            ThreadState::current()->registerStaticPersistentNode(m_persistentNode, &PersistentHeapCollectionBase<Collection>::clearPersistentNode);
             LEAK_SANITIZER_IGNORE_OBJECT(this);
         }
         return this;
     }
-#endif
 
 private:
 
+    // Used when the registered PersistentNode of this object is
+    // released during ThreadState shutdown, clearing the association.
+    static void clearPersistentNode(void *self)
+    {
+        (reinterpret_cast<PersistentHeapCollectionBase<Collection>*>(self))->uninitialize();

[haraken, 2016/04/25 08:39:23]

Why does PersistentHeapCollectionBase need to call uninitialize() while
PersistentBase doesn't need to call it?

[sof, 2016/04/25 08:53:04]

It also needs to clear the connection; see
PersistentRegion::releasePersistentNode() calling clear() on those.

+    }
+
     NO_LAZY_SWEEP_SANITIZE_ADDRESS
     void initialize()
     {
         // FIXME: Derive affinity based on the collection.
         ThreadState* state = ThreadState::current();
         ASSERT(state->checkThread());
         m_persistentNode = state->getPersistentRegion()->allocatePersistentNode(this, TraceMethodDelegate<PersistentHeapCollectionBase<Collection>, &PersistentHeapCollectionBase<Collection>::trace>::trampoline);
 #if ENABLE(ASSERT)
         m_state = state;
 #endif
     }
 
     void uninitialize()
     {
+        if (!m_persistentNode)

[haraken, 2016/04/25 08:39:23]

Just curious, why do we need this check?

[sof, 2016/04/25 08:53:05]

If you have a PersistentCollectionBase stuck in TLS (via ThreadSpecific<>),
it'll destruct the object. That musn't touch m_persistentNode.

+            return;
         ThreadState* state = ThreadState::current();
         ASSERT(state->checkThread());
         // Persistent handle must be created and destructed in the same thread.
         ASSERT(m_state == state);
-        state->getPersistentRegion()->freePersistentNode(m_persistentNode);
+        state->freePersistentNode(m_persistentNode);
+        m_persistentNode = nullptr;
     }
 
     PersistentNode* m_persistentNode;
 #if ENABLE(ASSERT)
     ThreadState* m_state;
 #endif
 };
 
 template<
     typename KeyArg,
@@ skipping 626 matching lines @@
     // into it.
     //
     // TODO(sof): remove this hack once wtf/Functional.h can also work with a type like
     // CrossThreadWeakPersistent<>.
     static WeakPtr<T> unwrap(const StorageType& value) { return WeakPtr<T>(WeakReference<T>::create(value.get())); }
 };
 
 } // namespace WTF
 
 #endif