TrackedObjects assumes you can use a "TLS slot" of -1 to indicate uninitialized.  This isn't true for the pthread_key_t type, which is unsigned on Linux and reportedly a struct on Macs.  This change modifies the Slot type to be a struct containing an "initialized" flag.


git-svn-id: svn://svn.chromium.org/chrome/trunk/src@1113 0039d316-1c4b-4281-b951-d872f2087c98


CrOS-Libchrome-Original-Commit: c3ec35638ea6594b6bdbac7ff92c38add8984623
diff --git a/base/message_loop.cc b/base/message_loop.cc
index caec847..bdf99f0 100644
--- a/base/message_loop.cc
+++ b/base/message_loop.cc
@@ -39,7 +39,9 @@
 // a TLS index to the message loop for the current thread
 // Note that if we start doing complex stuff in other static initializers
 // this could cause problems.
-/*static*/ TLSSlot MessageLoop::tls_index_ = ThreadLocalStorage::Alloc();
+// TODO(evanm): this shouldn't rely on static initialization.
+// static
+TLSSlot MessageLoop::tls_index_;
 
 //------------------------------------------------------------------------------
 
@@ -83,7 +85,7 @@
       exception_restoration_(false),
       state_(NULL) {
   DCHECK(!current()) << "should only have one message loop per thread";
-  ThreadLocalStorage::Set(tls_index_, this);
+  tls_index_.Set(this);
   // TODO(darin): Generalize this to support instantiating different pumps.
 #if defined(OS_WIN)
   pump_ = new base::MessagePumpWin();
@@ -100,7 +102,7 @@
                     WillDestroyCurrentMessageLoop());
 
   // OK, now make it so that no one can find us.
-  ThreadLocalStorage::Set(tls_index_, NULL);
+  tls_index_.Set(NULL);
 
   DCHECK(!state_);
 
diff --git a/base/message_loop.h b/base/message_loop.h
index 3821519..e11b73f 100644
--- a/base/message_loop.h
+++ b/base/message_loop.h
@@ -194,7 +194,7 @@
 
   // Returns the MessageLoop object for the current thread, or null if none.
   static MessageLoop* current() {
-    return static_cast<MessageLoop*>(ThreadLocalStorage::Get(tls_index_));
+    return static_cast<MessageLoop*>(tls_index_.Get());
   }
 
   // Returns the TimerManager object for the current thread.
diff --git a/base/stats_table.cc b/base/stats_table.cc
index 23c736a..8b10662 100644
--- a/base/stats_table.cc
+++ b/base/stats_table.cc
@@ -254,7 +254,7 @@
 
 StatsTable::StatsTable(const std::wstring& name, int max_threads,
                        int max_counters)
-    : tls_index_(ThreadLocalStorage::Alloc(SlotReturnFunction)) {
+    : tls_index_(SlotReturnFunction) {
   int table_size =
     AlignedSize(sizeof(TableHeader)) +
     AlignedSize((max_counters * sizeof(wchar_t) * kMaxCounterNameLength)) +
@@ -285,7 +285,7 @@
 
   // Return ThreadLocalStorage.  At this point, if any registered threads
   // still exist, they cannot Unregister.
-  ThreadLocalStorage::Free(tls_index_);
+  tls_index_.Free();
 
   // Cleanup our shared memory.
   delete impl_;
@@ -329,13 +329,13 @@
   StatsTableTLSData* data = new StatsTableTLSData;
   data->table = this;
   data->slot = slot;
-  ThreadLocalStorage::Set(tls_index_, data);
+  tls_index_.Set(data);
   return slot;
 }
 
 StatsTableTLSData* StatsTable::GetTLSData() const {
   StatsTableTLSData* data =
-    static_cast<StatsTableTLSData*>(ThreadLocalStorage::Get(tls_index_));
+    static_cast<StatsTableTLSData*>(tls_index_.Get());
   if (!data)
     return NULL;
 
@@ -355,7 +355,7 @@
   *name = L'\0';
 
   // Remove the calling thread's TLS so that it cannot use the slot.
-  ThreadLocalStorage::Set(tls_index_, NULL);
+  tls_index_.Set(NULL);
   delete data;
 }
 
diff --git a/base/thread.cc b/base/thread.cc
index 629c7e0..6998deb 100644
--- a/base/thread.cc
+++ b/base/thread.cc
@@ -69,7 +69,6 @@
       thread_id_(0),
       message_loop_(NULL),
       name_(name) {
-  DCHECK(tls_index_) << "static initializer failed";
 }
 
 Thread::~Thread() {
@@ -82,18 +81,19 @@
 // Thread to setup and run a MessageLoop.
 // Note that if we start doing complex stuff in other static initializers
 // this could cause problems.
-TLSSlot Thread::tls_index_ = ThreadLocalStorage::Alloc();
+// TODO(evanm): this shouldn't rely on static initialization.
+TLSSlot Thread::tls_index_;
 
 void Thread::SetThreadWasQuitProperly(bool flag) {
 #ifndef NDEBUG
-  ThreadLocalStorage::Set(tls_index_, reinterpret_cast<void*>(flag));
+  tls_index_.Set(reinterpret_cast<void*>(flag));
 #endif
 }
 
 bool Thread::GetThreadWasQuitProperly() {
   bool quit_properly = true;
 #ifndef NDEBUG
-  quit_properly = (ThreadLocalStorage::Get(tls_index_) != 0);
+  quit_properly = (tls_index_.Get() != 0);
 #endif
   return quit_properly;
 }
diff --git a/base/thread_local_storage.h b/base/thread_local_storage.h
index 245a73f..fba90ae 100644
--- a/base/thread_local_storage.h
+++ b/base/thread_local_storage.h
@@ -32,43 +32,62 @@
 
 #include "base/basictypes.h"
 
-#if defined(OS_WIN)
-typedef int TLSSlot;
-#elif defined(OS_POSIX)
+#if defined(OS_POSIX)
 #include <pthread.h>
-typedef pthread_key_t TLSSlot;
-#endif  // OS_*
+#endif
 
-// Wrapper for thread local storage.  This class doesn't
-// do much except provide an API for portability later.
+// Wrapper for thread local storage.  This class doesn't do much except provide
+// an API for portability.
 class ThreadLocalStorage {
  public:
-  // Prototype for the TLS destructor function, which can be
-  // optionally used to cleanup thread local storage on
-  // thread exit.  'value' is the data that is stored
-  // in thread local storage.
+
+  // Prototype for the TLS destructor function, which can be optionally used to
+  // cleanup thread local storage on thread exit.  'value' is the data that is
+  // stored in thread local storage.
   typedef void (*TLSDestructorFunc)(void* value);
 
-  // Allocate a TLS 'slot'.
-  // 'destructor' is a pointer to a function to perform
-  // per-thread cleanup of this object.  If set to NULL,
-  // no cleanup is done for this TLS slot.
-  // Returns an index > 0 on success, or -1 on failure.
-  static TLSSlot Alloc(TLSDestructorFunc destructor = NULL);
+  // A key representing one value stored in TLS.
+  class Slot {
+   public:
+    Slot(TLSDestructorFunc destructor = NULL);
 
-  // Free a previously allocated TLS 'slot'.
-  // If a destructor was set for this slot, removes
-  // the destructor so that remaining threads exiting
-  // will not free data.
-  static void Free(TLSSlot slot);
+    // This constructor should be used for statics.
+    // It returns an uninitialized Slot.
+    explicit Slot(base::LinkerInitialized x) {}
 
-  // Get the thread-local value stored in slot 'slot'.
-  // Values are guaranteed to initially be zero.
-  static void* Get(TLSSlot slot);
+    // Set up the TLS slot.  Called by the constructor.
+    // 'destructor' is a pointer to a function to perform per-thread cleanup of
+    // this object.  If set to NULL, no cleanup is done for this TLS slot.
+    // Returns false on error.
+    bool Initialize(TLSDestructorFunc destructor);
 
-  // Set the thread-local value stored in slot 'slot' to
-  // value 'value'.
-  static void Set(TLSSlot slot, void* value);
+    // Free a previously allocated TLS 'slot'.
+    // If a destructor was set for this slot, removes
+    // the destructor so that remaining threads exiting
+    // will not free data.
+    void Free();
+
+    // Get the thread-local value stored in slot 'slot'.
+    // Values are guaranteed to initially be zero.
+    void* Get() const;
+
+    // Set the thread-local value stored in slot 'slot' to
+    // value 'value'.
+    void Set(void* value);
+
+    bool initialized() const { return initialized_; }
+
+   private:
+    // The internals of this struct should be considered private.
+    bool initialized_;
+#if defined(OS_WIN)
+    int slot_;
+#elif defined(OS_POSIX)
+    pthread_key_t key_;
+#endif
+
+    DISALLOW_COPY_AND_ASSIGN(Slot);
+  };
 
 #if defined(OS_WIN)
   // Function called when on thread exit to call TLS
@@ -90,7 +109,11 @@
   static TLSDestructorFunc tls_destructors_[kThreadLocalStorageSize];
 #endif  // OS_WIN
 
-  DISALLOW_EVIL_CONSTRUCTORS(ThreadLocalStorage);
+  DISALLOW_COPY_AND_ASSIGN(ThreadLocalStorage);
 };
 
+// Temporary backwards-compatible name.
+// TODO(evanm): replace all usage of TLSSlot.
+typedef ThreadLocalStorage::Slot TLSSlot;
+
 #endif  // BASE_THREAD_LOCAL_STORAGE_H_
diff --git a/base/thread_local_storage_posix.cc b/base/thread_local_storage_posix.cc
index bcc982f..7c24e59 100644
--- a/base/thread_local_storage_posix.cc
+++ b/base/thread_local_storage_posix.cc
@@ -31,23 +31,39 @@
 
 #include "base/logging.h"
 
-TLSSlot ThreadLocalStorage::Alloc(TLSDestructorFunc destructor) {
-  TLSSlot key;
-  int error = pthread_key_create(&key, destructor);
+ThreadLocalStorage::Slot::Slot(TLSDestructorFunc destructor)
+    : initialized_(false) {
+  Initialize(destructor);
+}
+
+bool ThreadLocalStorage::Slot::Initialize(TLSDestructorFunc destructor) {
+  DCHECK(!initialized_);
+  int error = pthread_key_create(&key_, destructor);
+  if (error) {
+    NOTREACHED();
+    return false;
+  }
+  
+  initialized_ = true;
+  return true;
+}
+
+void ThreadLocalStorage::Slot::Free() {
+  DCHECK(initialized_);
+  int error = pthread_key_delete(key_);
   if (error)
     NOTREACHED();
-  
-  return key;
+  initialized_ = false;
 }
 
-void ThreadLocalStorage::Free(TLSSlot slot) {
-  pthread_key_delete(slot);
+void* ThreadLocalStorage::Slot::Get() const {
+  DCHECK(initialized_);
+  return pthread_getspecific(key_);
 }
 
-void* ThreadLocalStorage::Get(TLSSlot slot) {
-  return pthread_getspecific(slot);
-}
-
-void ThreadLocalStorage::Set(TLSSlot slot, void* value) {
-  pthread_setspecific(slot, value);
+void ThreadLocalStorage::Slot::Set(void* value) {
+  DCHECK(initialized_);
+  int error = pthread_setspecific(key_, value);
+  if (error)
+    NOTREACHED();
 }
diff --git a/base/tracked_objects.cc b/base/tracked_objects.cc
index b683a87..83a7fbd 100644
--- a/base/tracked_objects.cc
+++ b/base/tracked_objects.cc
@@ -35,9 +35,9 @@
 
 namespace tracked_objects {
 
-// a TLS index to the TrackRegistry for the current thread.
+// A TLS slot to the TrackRegistry for the current thread.
 // static
-TLSSlot ThreadData::tls_index_ = -1;
+TLSSlot ThreadData::tls_index_(base::LINKER_INITIALIZED);
 
 //------------------------------------------------------------------------------
 // Death data tallies durations when a death takes place.
@@ -104,15 +104,14 @@
 // static
 ThreadData::Status ThreadData::status_ = ThreadData::UNINITIALIZED;
 
-ThreadData::ThreadData() :  message_loop_(MessageLoop::current()) {}
+ThreadData::ThreadData() : message_loop_(MessageLoop::current()) {}
 
 // static
 ThreadData* ThreadData::current() {
-  if (-1 == tls_index_)
-    return NULL;  // not yet initialized.
+  if (!tls_index_.initialized())
+    return NULL;
 
-  ThreadData* registry =
-      static_cast<ThreadData*>(ThreadLocalStorage::Get(tls_index_));
+  ThreadData* registry = static_cast<ThreadData*>(tls_index_.Get());
   if (!registry) {
     // We have to create a new registry for ThreadData.
     bool too_late_to_create = false;
@@ -132,7 +131,7 @@
       delete registry;
       registry = NULL;
     } else {
-      ThreadLocalStorage::Set(tls_index_, registry);
+      tls_index_.Set(registry);
     }
   }
   return registry;
@@ -344,11 +343,9 @@
     status_ = SHUTDOWN;
     return true;
   }
-  TLSSlot tls_index = ThreadLocalStorage::Alloc();
   AutoLock lock(list_lock_);
   DCHECK(status_ == UNINITIALIZED);
-  tls_index_ = tls_index;
-  CHECK(-1 != tls_index_);
+  CHECK(tls_index_.Initialize(NULL));
   status_ = ACTIVE;
   return true;
 }
@@ -401,9 +398,9 @@
     delete next_thread_data;  // Includes all Death Records.
   }
 
-  CHECK(-1 != tls_index_);
-  ThreadLocalStorage::Free(tls_index_);
-  tls_index_ = -1;
+  CHECK(tls_index_.initialized());
+  tls_index_.Free();
+  DCHECK(!tls_index_.initialized());
   status_ = UNINITIALIZED;
 }