Do not rely on uniquiness of pthread_t

src/isolate.cc

Index: src/isolate.cc
diff --git a/src/isolate.cc b/src/isolate.cc
index b8a7fb75ac6c56733977c3274990e5b33164728b..42a4f57fce077f697f0baa9da84df227081c42ee 100644
--- a/src/isolate.cc
+++ b/src/isolate.cc
@@ -54,6 +54,26 @@
 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
@@ -245,7 +265,6 @@ 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 {
@@ -265,20 +284,12 @@ static IsolateInitializer* EnsureDefaultIsolateAllocated() {
 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_);
@@ -292,11 +303,7 @@ Isolate::PerIsolateThreadData* Isolate::AllocatePerIsolateThreadData(
 
 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_);
@@ -361,7 +368,8 @@ Isolate::ThreadDataTable::ThreadDataTable()
 
 
 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;
   }
@@ -383,7 +391,8 @@ void Isolate::ThreadDataTable::Remove(PerIsolateThreadData* data) {
 }
 
 
-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);
@@ -833,8 +842,8 @@ void Isolate::Enter() {
       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;
@@ -872,8 +881,8 @@ void Isolate::Enter() {
 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;