Do not rely on uniquiness of pthread_t

src/isolate.cc

 // Copyright 2006-2010 the V8 project authors. 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
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 36 matching lines @@
 #include "serialize.h"
 #include "simulator.h"
 #include "spaces.h"
 #include "stub-cache.h"
 #include "version.h"
 
 
 namespace v8 {
 namespace internal {
 
+Mutex* ThreadId::thread_id_process_wide_mutex_ = OS::CreateMutex();

[Vitaly Repeshko, 2011/04/11 19:28:52]

I'm worried that this could be accessed before the mutex is statically
initialized. I see two ways out: 1) add "static bool is_initialized = true;" and
check that it's true before entering thread id code (this will show if there're
any problematic usages) or 2) use atomic ops (Increment) and avoid using the
mutex completely. I'm in favor of (2).

[Dmitry Lomov, 2011/04/11 20:08:35]

I think we have the same issue with process_wide_mutex in Isolate class (see
below in this file). So I think this is benign. However atomics are cleaner and
faster - I'll change the impl  

On 2011/04/11 19:28:52, Vitaly Repeshko wrote:

[Dmitry Lomov, 2011/04/11 21:41:14]

Done.

+int ThreadId::highest_thread_id_ = 0;
+
+int ThreadId::AllocateThreadId() {
+  int new_id;
+  {
+    ScopedLock lock(thread_id_process_wide_mutex_);
+    new_id = ++highest_thread_id_;
+  }
+  return new_id;
+}
+
+int ThreadId::GetCurrentThreadId() {
+  int thread_id = Thread::GetThreadLocalInt(Isolate::thread_id_key_);
+  if (thread_id == 0) {
+    thread_id = AllocateThreadId();
+    Thread::SetThreadLocalInt(Isolate::thread_id_key_, thread_id);
+  }
+  return thread_id;
+}
 
 // Create a dummy thread that will wait forever on a semaphore. The only
 // purpose for this thread is to have some stack area to save essential data
 // into for use by a stacks only core dump (aka minidump).
 class PreallocatedMemoryThread: public Thread {
  public:
   char* data() {
     if (data_ready_semaphore_ != NULL) {
       // Initial access is guarded until the data has been published.
       data_ready_semaphore_->Wait();
@@ skipping 171 matching lines @@
   storage->LinkTo(&free_list_);
 }
 
 
 Isolate* Isolate::default_isolate_ = NULL;
 Thread::LocalStorageKey Isolate::isolate_key_;
 Thread::LocalStorageKey Isolate::thread_id_key_;
 Thread::LocalStorageKey Isolate::per_isolate_thread_data_key_;
 Mutex* Isolate::process_wide_mutex_ = OS::CreateMutex();
 Isolate::ThreadDataTable* Isolate::thread_data_table_ = NULL;
-Isolate::ThreadId Isolate::highest_thread_id_ = 0;
 
 
 class IsolateInitializer {
  public:
   IsolateInitializer() {
     Isolate::EnsureDefaultIsolate();
   }
 };
 
 static IsolateInitializer* EnsureDefaultIsolateAllocated() {
   // TODO(isolates): Use the system threading API to do this once?
   static IsolateInitializer static_initializer;
   return &static_initializer;
 }
 
 // This variable only needed to trigger static intialization.
 static IsolateInitializer* static_initializer = EnsureDefaultIsolateAllocated();
 
 
-Isolate::ThreadId Isolate::AllocateThreadId() {
-  ThreadId new_id;
-  {
-    ScopedLock lock(process_wide_mutex_);
-    new_id = ++highest_thread_id_;
-  }
-  return new_id;
-}
+
 
 
 Isolate::PerIsolateThreadData* Isolate::AllocatePerIsolateThreadData(
     ThreadId thread_id) {
-  ASSERT(thread_id != 0);
-  ASSERT(Thread::GetThreadLocalInt(thread_id_key_) == thread_id);
+  ASSERT(!thread_id.Equals(ThreadId::Invalid()));
   PerIsolateThreadData* per_thread = new PerIsolateThreadData(this, thread_id);
   {
     ScopedLock lock(process_wide_mutex_);
     ASSERT(thread_data_table_->Lookup(this, thread_id) == NULL);
     thread_data_table_->Insert(per_thread);
     ASSERT(thread_data_table_->Lookup(this, thread_id) == per_thread);
   }
   return per_thread;
 }
 
 
 Isolate::PerIsolateThreadData*
     Isolate::FindOrAllocatePerThreadDataForThisThread() {
-  ThreadId thread_id = Thread::GetThreadLocalInt(thread_id_key_);
-  if (thread_id == 0) {
-    thread_id = AllocateThreadId();
-    Thread::SetThreadLocalInt(thread_id_key_, thread_id);
-  }
+  ThreadId thread_id = ThreadId::Current();
   PerIsolateThreadData* per_thread = NULL;
   {
     ScopedLock lock(process_wide_mutex_);
     per_thread = thread_data_table_->Lookup(this, thread_id);
     if (per_thread == NULL) {
       per_thread = AllocatePerIsolateThreadData(thread_id);
     }
   }
   return per_thread;
 }
@@ skipping 44 matching lines @@
   return default_isolate_;
 }
 
 
 Isolate::ThreadDataTable::ThreadDataTable()
     : list_(NULL) {
 }
 
 
 Isolate::PerIsolateThreadData*
-    Isolate::ThreadDataTable::Lookup(Isolate* isolate, ThreadId thread_id) {
+    Isolate::ThreadDataTable::Lookup(Isolate* isolate,
+                                     ThreadId thread_id) {
   for (PerIsolateThreadData* data = list_; data != NULL; data = data->next_) {
     if (data->Matches(isolate, thread_id)) return data;
   }
   return NULL;
 }
 
 
 void Isolate::ThreadDataTable::Insert(Isolate::PerIsolateThreadData* data) {
   if (list_ != NULL) list_->prev_ = data;
   data->next_ = list_;
   list_ = data;
 }
 
 
 void Isolate::ThreadDataTable::Remove(PerIsolateThreadData* data) {
   if (list_ == data) list_ = data->next_;
   if (data->next_ != NULL) data->next_->prev_ = data->prev_;
   if (data->prev_ != NULL) data->prev_->next_ = data->next_;
 }
 
 
-void Isolate::ThreadDataTable::Remove(Isolate* isolate, ThreadId thread_id) {
+void Isolate::ThreadDataTable::Remove(Isolate* isolate,
+                                      ThreadId thread_id) {
   PerIsolateThreadData* data = Lookup(isolate, thread_id);
   if (data != NULL) {
     Remove(data);
   }
 }
 
 
 #ifdef DEBUG
 #define TRACE_ISOLATE(tag)                                              \
   do {                                                                  \
@@ skipping 429 matching lines @@
 void Isolate::Enter() {
   Isolate* current_isolate = NULL;
   PerIsolateThreadData* current_data = CurrentPerIsolateThreadData();
   if (current_data != NULL) {
     current_isolate = current_data->isolate_;
     ASSERT(current_isolate != NULL);
     if (current_isolate == this) {
       ASSERT(Current() == this);
       ASSERT(entry_stack_ != NULL);
       ASSERT(entry_stack_->previous_thread_data == NULL ||
-             entry_stack_->previous_thread_data->thread_id() ==
-                 Thread::GetThreadLocalInt(thread_id_key_));
+             entry_stack_->previous_thread_data->thread_id().Equals(
+                 ThreadId::Current()));
       // Same thread re-enters the isolate, no need to re-init anything.
       entry_stack_->entry_count++;
       return;
     }
   }
 
   // Threads can have default isolate set into TLS as Current but not yet have
   // PerIsolateThreadData for it, as it requires more advanced phase of the
   // initialization. For example, a thread might be the one that system used for
   // static initializers - in this case the default isolate is set in TLS but
@@ skipping 17 matching lines @@
   CHECK(PreInit());
 
   // In case it's the first time some thread enters the isolate.
   set_thread_id(data->thread_id());
 }
 
 
 void Isolate::Exit() {
   ASSERT(entry_stack_ != NULL);
   ASSERT(entry_stack_->previous_thread_data == NULL ||
-         entry_stack_->previous_thread_data->thread_id() ==
-             Thread::GetThreadLocalInt(thread_id_key_));
+         entry_stack_->previous_thread_data->thread_id().Equals(
+             ThreadId::Current()));
 
   if (--entry_stack_->entry_count > 0) return;
 
   ASSERT(CurrentPerIsolateThreadData() != NULL);
   ASSERT(CurrentPerIsolateThreadData()->isolate_ == this);
 
   // Pop the stack.
   EntryStackItem* item = entry_stack_;
   entry_stack_ = item->previous_item;
 
@@ skipping 14 matching lines @@
 
 #ifdef DEBUG
 #define ISOLATE_FIELD_OFFSET(type, name, ignored)                       \
 const intptr_t Isolate::name##_debug_offset_ = OFFSET_OF(Isolate, name##_);
 ISOLATE_INIT_LIST(ISOLATE_FIELD_OFFSET)
 ISOLATE_INIT_ARRAY_LIST(ISOLATE_FIELD_OFFSET)
 #undef ISOLATE_FIELD_OFFSET
 #endif
 
 } }  // namespace v8::internal