Make ThreadLocalStorage more posix pthread compliant

base/threading/thread_local_storage_win.cc

 // Copyright (c) 2011 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.
 
 #include "base/threading/thread_local_storage.h"
 
 #include <windows.h>
 
 #include "base/logging.h"
 
@@ skipping 11 matching lines @@
 long ThreadLocalStorage::tls_key_ = TLS_OUT_OF_INDEXES;
 
 // tls_max_ is the high-water-mark of allocated thread local storage.
 // We intentionally skip 0 so that it is not confused with an
 // unallocated TLS slot.
 long ThreadLocalStorage::tls_max_ = 1;
 
 // An array of destructor function pointers for the slots.  If
 // a slot has a destructor, it will be stored in its corresponding
 // entry in this array.
-ThreadLocalStorage::TLSDestructorFunc
-  ThreadLocalStorage::tls_destructors_[kThreadLocalStorageSize];
+subtle::AtomicWord
+    ThreadLocalStorage::tls_destructors_[kThreadLocalStorageSize];
 
 void** ThreadLocalStorage::Initialize() {
   if (tls_key_ == TLS_OUT_OF_INDEXES) {
     long value = TlsAlloc();
     DCHECK(value != TLS_OUT_OF_INDEXES);
 
     // Atomically test-and-set the tls_key.  If the key is TLS_OUT_OF_INDEXES,
     // go ahead and set it.  Otherwise, do nothing, as another
     // thread already did our dirty work.
     if (InterlockedCompareExchange(&tls_key_, value, TLS_OUT_OF_INDEXES) !=
             TLS_OUT_OF_INDEXES) {
       // We've been shortcut. Another thread replaced tls_key_ first so we need
       // to destroy our index and use the one the other thread got first.
       TlsFree(value);
     }
   }
   DCHECK(!TlsGetValue(tls_key_));
 
-  // Create an array to store our data.
+  // Use use a stack allocated vector, so that we don't have dependence
+  // on our allocator until our service is in place.  (i.e., don't
+  // even call new until after we're setup)

[willchan no longer on Chromium, 2011/11/28 20:58:28]

You should explain that this is because an allocator may use thread local
storage.

[jar (doing other things), 2011/11/29 02:32:45]

Done.

+  void* stack_allocated_tls_data[kThreadLocalStorageSize];
+  memset(stack_allocated_tls_data, 0, sizeof(stack_allocated_tls_data));
+  // Ensure that any rentrant calls change the temp version.
+  TlsSetValue(tls_key_, stack_allocated_tls_data);
+
+  // Allocate an array to store our data.
   void** tls_data = new void*[kThreadLocalStorageSize];
-  memset(tls_data, 0, sizeof(void*[kThreadLocalStorageSize]));
+  memcpy(tls_data, stack_allocated_tls_data, sizeof(stack_allocated_tls_data));
   TlsSetValue(tls_key_, tls_data);
   return tls_data;
 }
 
 ThreadLocalStorage::Slot::Slot(TLSDestructorFunc destructor)
     : initialized_(false),
       slot_(0) {
   Initialize(destructor);
 }
 
 bool ThreadLocalStorage::Slot::Initialize(TLSDestructorFunc destructor) {
   if (tls_key_ == TLS_OUT_OF_INDEXES || !TlsGetValue(tls_key_))
     ThreadLocalStorage::Initialize();
 
   // Grab a new slot.
   slot_ = InterlockedIncrement(&tls_max_) - 1;
+  DCHECK_GT(slot_, 0);
   if (slot_ >= kThreadLocalStorageSize) {
     NOTREACHED();
     return false;
   }
 
-  // Setup our destructor.
-  tls_destructors_[slot_] = destructor;
+  // Setup our destructor.  Make sure that any thread that is dostroyed can see

[willchan no longer on Chromium, 2011/11/28 21:04:32]

s/dostroyed/destroyed/

[jar (doing other things), 2011/11/29 02:32:45]

Done.

+  // this new setting.
+  subtle::NoBarrier_Store(&tls_destructors_[slot_],

[willchan no longer on Chromium, 2011/11/28 21:04:32]

It's not obvious to me that the NoBarrier variants are what you want to use
here. I would expect to do an Acquire_Load() and Release_Store(). Can you
explain why it's ok to use NoBarrier_Store() and NoBarrier_Load()?

[jar (doing other things), 2011/11/29 02:32:45]

As per our discussion, I added a pile of comments justifying this.

On 2011/11/28 21:04:32, willchan wrote:

+                          reinterpret_cast<subtle::AtomicWord>(destructor));
   initialized_ = true;
   return true;
 }
 
 void ThreadLocalStorage::Slot::Free() {
   // At this time, we don't reclaim old indices for TLS slots.
   // So all we need to do is wipe the destructor.
-  tls_destructors_[slot_] = NULL;
+  DCHECK_GT(slot_, 0);
+  DCHECK_LT(slot_, kThreadLocalStorageSize);
+  subtle::NoBarrier_Store(&tls_destructors_[slot_], NULL);
+  slot_ = 0;
   initialized_ = false;
 }
 
 void* ThreadLocalStorage::Slot::Get() const {
   void** tls_data = static_cast<void**>(TlsGetValue(tls_key_));
   if (!tls_data)
     tls_data = ThreadLocalStorage::Initialize();
-  DCHECK_GE(slot_, 0);
+  DCHECK_GT(slot_, 0);
   DCHECK_LT(slot_, kThreadLocalStorageSize);
   return tls_data[slot_];
 }
 
 void ThreadLocalStorage::Slot::Set(void* value) {
   void** tls_data = static_cast<void**>(TlsGetValue(tls_key_));
   if (!tls_data)
     tls_data = ThreadLocalStorage::Initialize();
-  DCHECK_GE(slot_, 0);
+  DCHECK_GT(slot_, 0);
   DCHECK_LT(slot_, kThreadLocalStorageSize);
   tls_data[slot_] = value;
 }
 
 void ThreadLocalStorage::ThreadExit() {
   if (tls_key_ == TLS_OUT_OF_INDEXES)
     return;
 
   void** tls_data = static_cast<void**>(TlsGetValue(tls_key_));
-
   // Maybe we have never initialized TLS for this thread.
   if (!tls_data)
     return;
 
-  for (int slot = 0; slot < tls_max_; slot++) {
-    if (tls_destructors_[slot] != NULL) {
-      void* value = tls_data[slot];
-      tls_destructors_[slot](value);
+  // Switch to using a stack allocated vector, so that we don't have dependence
+  // on our allocator after we have called all tls_destructors_.  (i.e., don't
+  // even call delete[] after we're done with destructors.)

[willchan no longer on Chromium, 2011/11/28 20:58:28]

You should again explain that this is because the allocator may depend on thread
local data structures that may no longer exist.

[jar (doing other things), 2011/11/29 02:32:45]

Done.

+  void* stack_allocated_tls_data[kThreadLocalStorageSize];
+  memcpy(stack_allocated_tls_data, tls_data, sizeof(stack_allocated_tls_data));
+  // Ensure that any rentrant calls change the temp version.
+  TlsSetValue(tls_key_, stack_allocated_tls_data);
+  delete[] tls_data;  // Our last dependence on an allocator.
+
+  int remaining_attempts = PTHREAD_DESTRUCTOR_ITERATIONS;
+  bool need_to_scan_destructors = true;
+  while (need_to_scan_destructors) {
+    need_to_scan_destructors = false;
+    // Try to destroy the first-created-slot (which is slot 1) in our last
+    // destructor call.  That user was able to function, and define a slot with
+    // no other services running, so perhaps it is a basic service (like an
+    // allocator) and should also be destroyed last.
+    for (int slot = tls_max_ - 1; slot > 0; --slot) {
+      void* value = stack_allocated_tls_data[slot];
+      if (value == NULL)
+        continue;
+      TLSDestructorFunc destructor =
+          reinterpret_cast<ThreadLocalStorage::TLSDestructorFunc>(
+              subtle::NoBarrier_Load(&tls_destructors_[slot]));
+      if (destructor == NULL)
+        continue;
+      stack_allocated_tls_data[slot] = NULL;  // pre-clear the slot.
+      destructor(value);
+      // Any destructor might have called a different service, which then set
+      // a different slot to a non-NULL value.  Hence we need to check
+      // the whole vector again.  This is a pthread standard.
+      need_to_scan_destructors = true;
+    }
+    if (--remaining_attempts <= 0) {
+      NOTREACHED();  // Destructors might not have been called.
+      break;
     }
   }
 
-  delete[] tls_data;
-
-  // In case there are other "onexit" handlers...
+  // Remove our stack allocated vector.
   TlsSetValue(tls_key_, NULL);
 }
 
 }  // namespace base
 
 // Thread Termination Callbacks.
 // Windows doesn't support a per-thread destructor with its
 // TLS primitives.  So, we build it manually by inserting a
 // function to be called on each thread's exit.
 // This magic is from http://www.codeproject.com/threads/tls.asp
@@ skipping 55 matching lines @@
 #else  // _WIN64
 
 #pragma data_seg(".CRT$XLB")
 PIMAGE_TLS_CALLBACK p_thread_callback_base = OnThreadExit;
 
 // Reset the default section.
 #pragma data_seg()
 
 #endif  // _WIN64
 }  // extern "C"