Do not rely on uniqueness of pthread_t

Patch by Dmitry Lomov.

pthreads implementations are free to reuse pthread_t (thread id) after
the thread has died. This change gets rid of ThreadHandle class and
replaces it with v8-managed thread identifiers.


git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@7575 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/api.cc

@@ -4625,7 +4625,7 @@ int V8::GetLogLines(int from_pos, char* dest_buf, int max_size) {
 int V8::GetCurrentThreadId() {
   i::Isolate* isolate = i::Isolate::Current();
   EnsureInitializedForIsolate(isolate, "V8::GetCurrentThreadId()");
-  return isolate->thread_id();
+  return isolate->thread_id().ToInteger();
 }
 
 
@@ -4636,10 +4636,11 @@ void V8::TerminateExecution(int thread_id) {
   // If the thread_id identifies the current thread just terminate
   // execution right away.  Otherwise, ask the thread manager to
   // terminate the thread with the given id if any.
-  if (thread_id == isolate->thread_id()) {
+  i::ThreadId internal_tid = i::ThreadId::FromInteger(thread_id);
+  if (isolate->thread_id().Equals(internal_tid)) {
     isolate->stack_guard()->TerminateExecution();
   } else {
-    isolate->thread_manager()->TerminateExecution(thread_id);
+    isolate->thread_manager()->TerminateExecution(internal_tid);
   }
 }
 

src/isolate.cc

@@ -54,6 +54,21 @@
 namespace v8 {
 namespace internal {
 
+Atomic32 ThreadId::highest_thread_id_ = 0;
+
+int ThreadId::AllocateThreadId() {
+  int new_id = NoBarrier_AtomicIncrement(&highest_thread_id_, 1);
+  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 +260,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 +279,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 +298,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 +363,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 +386,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 +837,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 +876,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;
 

src/isolate.h

@@ -136,6 +136,53 @@ typedef ZoneList<Handle<Object> > ZoneObjectList;
 #endif
 
 
+// Platform-independent, reliable thread identifier.
+class ThreadId {
+ public:
+  // Creates an invalid ThreadId.
+  ThreadId() : id_(kInvalidId) {}
+
+  // Returns ThreadId for current thread.
+  static ThreadId Current() { return ThreadId(GetCurrentThreadId()); }
+
+  // Returns invalid ThreadId (guaranteed not to be equal to any thread).
+  static ThreadId Invalid() { return ThreadId(kInvalidId); }
+
+  // Compares ThreadIds for equality.
+  INLINE(bool Equals(const ThreadId& other) const) {
+    return id_ == other.id_;
+  }
+
+  // Checks whether this ThreadId refers to any thread.
+  INLINE(bool IsValid() const) {
+    return id_ != kInvalidId;
+  }
+
+  // Converts ThreadId to an integer representation
+  // (required for public API: V8::V8::GetCurrentThreadId).
+  int ToInteger() const { return id_; }
+
+  // Converts ThreadId to an integer representation
+  // (required for public API: V8::V8::TerminateExecution).
+  static ThreadId FromInteger(int id) { return ThreadId(id); }
+
+ private:
+  static const int kInvalidId = -1;
+
+  explicit ThreadId(int id) : id_(id) {}
+
+  static int AllocateThreadId();
+
+  static int GetCurrentThreadId();
+
+  int id_;
+
+  static Atomic32 highest_thread_id_;
+
+  friend class Isolate;
+};
+
+
 class ThreadLocalTop BASE_EMBEDDED {
  public:
   // Initialize the thread data.
@@ -176,7 +223,7 @@ class ThreadLocalTop BASE_EMBEDDED {
   // The context where the current execution method is created and for variable
   // lookups.
   Context* context_;
-  int thread_id_;
+  ThreadId thread_id_;
   MaybeObject* pending_exception_;
   bool has_pending_message_;
   Object* pending_message_obj_;
@@ -329,8 +376,6 @@ class Isolate {
  public:
   ~Isolate();
 
-  typedef int ThreadId;
-
   // A thread has a PerIsolateThreadData instance for each isolate that it has
   // entered. That instance is allocated when the isolate is initially entered
   // and reused on subsequent entries.
@@ -363,7 +408,7 @@ class Isolate {
 #endif
 
     bool Matches(Isolate* isolate, ThreadId thread_id) const {
-      return isolate_ == isolate && thread_id_ == thread_id;
+      return isolate_ == isolate && thread_id_.Equals(thread_id);
     }
 
    private:
@@ -455,9 +500,6 @@ class Isolate {
     return thread_id_key_;
   }
 
-  // Atomically allocates a new thread ID.
-  static ThreadId AllocateThreadId();
-
   // If a client attempts to create a Locker without specifying an isolate,
   // we assume that the client is using legacy behavior. Set up the current
   // thread to be inside the implicit isolate (or fail a check if we have
@@ -483,8 +525,8 @@ class Isolate {
   }
 
   // Access to current thread id.
-  int thread_id() { return thread_local_top_.thread_id_; }
-  void set_thread_id(int id) { thread_local_top_.thread_id_ = id; }
+  ThreadId thread_id() { return thread_local_top_.thread_id_; }
+  void set_thread_id(ThreadId id) { thread_local_top_.thread_id_ = id; }
 
   // Interface to pending exception.
   MaybeObject* pending_exception() {
@@ -996,7 +1038,6 @@ class Isolate {
   static Thread::LocalStorageKey thread_id_key_;
   static Isolate* default_isolate_;
   static ThreadDataTable* thread_data_table_;
-  static ThreadId highest_thread_id_;
 
   bool PreInit();
 
@@ -1156,6 +1197,7 @@ class Isolate {
 
   friend class ExecutionAccess;
   friend class IsolateInitializer;
+  friend class ThreadId;
   friend class v8::Isolate;
   friend class v8::Locker;
 

src/objects.cc

@@ -7635,7 +7635,6 @@ MaybeObject* JSObject::GetElementWithInterceptor(Object* receiver,
   Handle<InterceptorInfo> interceptor(GetIndexedInterceptor(), isolate);
   Handle<Object> this_handle(receiver, isolate);
   Handle<JSObject> holder_handle(this, isolate);
-
   if (!interceptor->getter()->IsUndefined()) {
     v8::IndexedPropertyGetter getter =
         v8::ToCData<v8::IndexedPropertyGetter>(interceptor->getter());

src/platform-cygwin.cc

@@ -362,50 +362,17 @@ bool VirtualMemory::Uncommit(void* address, size_t size) {
 }
 
 
-class ThreadHandle::PlatformData : public Malloced {
+class Thread::PlatformData : public Malloced {
  public:
-  explicit PlatformData(ThreadHandle::Kind kind) {
-    Initialize(kind);
-  }
-
-  void Initialize(ThreadHandle::Kind kind) {
-    switch (kind) {
-      case ThreadHandle::SELF: thread_ = pthread_self(); break;
-      case ThreadHandle::INVALID: thread_ = kNoThread; break;
-    }
-  }
-
+  PlatformData() : thread_(kNoThread) {}
   pthread_t thread_;  // Thread handle for pthread.
 };
 
 
-ThreadHandle::ThreadHandle(Kind kind) {
-  data_ = new PlatformData(kind);
-}
-
-
-void ThreadHandle::Initialize(ThreadHandle::Kind kind) {
-  data_->Initialize(kind);
-}
-
-
-ThreadHandle::~ThreadHandle() {
-  delete data_;
-}
-
-
-bool ThreadHandle::IsSelf() const {
-  return pthread_equal(data_->thread_, pthread_self());
-}
-
-
-bool ThreadHandle::IsValid() const {
-  return data_->thread_ != kNoThread;
-}
 
 
 Thread::Thread(Isolate* isolate, const Options& options)
-    : ThreadHandle(ThreadHandle::INVALID),
+    : data_(new PlatformData),
       isolate_(isolate),
       stack_size_(options.stack_size) {
   set_name(options.name);
@@ -413,7 +380,7 @@ Thread::Thread(Isolate* isolate, const Options& options)
 
 
 Thread::Thread(Isolate* isolate, const char* name)
-    : ThreadHandle(ThreadHandle::INVALID),
+    : data_(new PlatformData),
       isolate_(isolate),
       stack_size_(0) {
   set_name(name);
@@ -421,6 +388,7 @@ Thread::Thread(Isolate* isolate, const char* name)
 
 
 Thread::~Thread() {
+  delete data_;
 }
 
 
@@ -429,8 +397,8 @@ static void* ThreadEntry(void* arg) {
   // This is also initialized by the first argument to pthread_create() but we
   // don't know which thread will run first (the original thread or the new
   // one) so we initialize it here too.
-  thread->thread_handle_data()->thread_ = pthread_self();
-  ASSERT(thread->IsValid());
+  thread->data()->thread_ = pthread_self();
+  ASSERT(thread->data()->thread_ != kNoThread);
   Thread::SetThreadLocal(Isolate::isolate_key(), thread->isolate());
   thread->Run();
   return NULL;
@@ -451,13 +419,13 @@ void Thread::Start() {
     pthread_attr_setstacksize(&attr, static_cast<size_t>(stack_size_));
     attr_ptr = &attr;
   }
-  pthread_create(&thread_handle_data()->thread_, attr_ptr, ThreadEntry, this);
-  ASSERT(IsValid());
+  pthread_create(&data_->thread_, attr_ptr, ThreadEntry, this);
+  ASSERT(data_->thread_ != kNoThread);
 }
 
 
 void Thread::Join() {
-  pthread_join(thread_handle_data()->thread_, NULL);
+  pthread_join(data_->thread_, NULL);
 }
 
 

src/platform-freebsd.cc

@@ -391,18 +391,8 @@ bool VirtualMemory::Uncommit(void* address, size_t size) {
 }
 
 
-class ThreadHandle::PlatformData : public Malloced {
+class Thread::PlatformData : public Malloced {
  public:
-  explicit PlatformData(ThreadHandle::Kind kind) {
-    Initialize(kind);
-  }
-
-  void Initialize(ThreadHandle::Kind kind) {
-    switch (kind) {
-      case ThreadHandle::SELF: thread_ = pthread_self(); break;
-      case ThreadHandle::INVALID: thread_ = kNoThread; break;
-    }
-  }
   pthread_t thread_;  // Thread handle for pthread.
 };
 
@@ -433,7 +423,7 @@ bool ThreadHandle::IsValid() const {
 
 
 Thread::Thread(Isolate* isolate, const Options& options)
-    : ThreadHandle(ThreadHandle::INVALID),
+    : data_(new PlatformData),
       isolate_(isolate),
       stack_size_(options.stack_size) {
   set_name(options.name);
@@ -441,7 +431,7 @@ Thread::Thread(Isolate* isolate, const Options& options)
 
 
 Thread::Thread(Isolate* isolate, const char* name)
-    : ThreadHandle(ThreadHandle::INVALID),
+    : data_(new PlatformData),
       isolate_(isolate),
       stack_size_(0) {
   set_name(name);
@@ -449,6 +439,7 @@ Thread::Thread(Isolate* isolate, const char* name)
 
 
 Thread::~Thread() {
+  delete data_;
 }
 
 
@@ -457,7 +448,7 @@ static void* ThreadEntry(void* arg) {
   // This is also initialized by the first argument to pthread_create() but we
   // don't know which thread will run first (the original thread or the new
   // one) so we initialize it here too.
-  thread->thread_handle_data()->thread_ = pthread_self();
+  thread_->data_->thread_ = pthread_self();
   ASSERT(thread->IsValid());
   Thread::SetThreadLocal(Isolate::isolate_key(), thread->isolate());
   thread->Run();

src/platform-linux.cc

@@ -588,50 +588,15 @@ bool VirtualMemory::Uncommit(void* address, size_t size) {
 }
 
 
-class ThreadHandle::PlatformData : public Malloced {
+class Thread::PlatformData : public Malloced {
  public:
-  explicit PlatformData(ThreadHandle::Kind kind) {
-    Initialize(kind);
-  }
-
-  void Initialize(ThreadHandle::Kind kind) {
-    switch (kind) {
-      case ThreadHandle::SELF: thread_ = pthread_self(); break;
-      case ThreadHandle::INVALID: thread_ = kNoThread; break;
-    }
-  }
+  PlatformData() : thread_(kNoThread) {}
 
   pthread_t thread_;  // Thread handle for pthread.
 };
 
-
-ThreadHandle::ThreadHandle(Kind kind) {
-  data_ = new PlatformData(kind);
-}
-
-
-void ThreadHandle::Initialize(ThreadHandle::Kind kind) {
-  data_->Initialize(kind);
-}
-
-
-ThreadHandle::~ThreadHandle() {
-  delete data_;
-}
-
-
-bool ThreadHandle::IsSelf() const {
-  return pthread_equal(data_->thread_, pthread_self());
-}
-
-
-bool ThreadHandle::IsValid() const {
-  return data_->thread_ != kNoThread;
-}
-
-
 Thread::Thread(Isolate* isolate, const Options& options)
-    : ThreadHandle(ThreadHandle::INVALID),
+    : data_(new PlatformData()),
       isolate_(isolate),
       stack_size_(options.stack_size) {
   set_name(options.name);
@@ -639,7 +604,7 @@ Thread::Thread(Isolate* isolate, const Options& options)
 
 
 Thread::Thread(Isolate* isolate, const char* name)
-    : ThreadHandle(ThreadHandle::INVALID),
+    : data_(new PlatformData()),
       isolate_(isolate),
       stack_size_(0) {
   set_name(name);
@@ -647,6 +612,7 @@ Thread::Thread(Isolate* isolate, const char* name)
 
 
 Thread::~Thread() {
+  delete data_;
 }
 
 
@@ -658,8 +624,8 @@ static void* ThreadEntry(void* arg) {
   prctl(PR_SET_NAME,
         reinterpret_cast<unsigned long>(thread->name()),  // NOLINT
         0, 0, 0);
-  thread->thread_handle_data()->thread_ = pthread_self();
-  ASSERT(thread->IsValid());
+  thread->data()->thread_ = pthread_self();
+  ASSERT(thread->data()->thread_ != kNoThread);
   Thread::SetThreadLocal(Isolate::isolate_key(), thread->isolate());
   thread->Run();
   return NULL;
@@ -680,13 +646,13 @@ void Thread::Start() {
     pthread_attr_setstacksize(&attr, static_cast<size_t>(stack_size_));
     attr_ptr = &attr;
   }
-  pthread_create(&thread_handle_data()->thread_, attr_ptr, ThreadEntry, this);
-  ASSERT(IsValid());
+  pthread_create(&data_->thread_, attr_ptr, ThreadEntry, this);
+  ASSERT(data_->thread_ != kNoThread);
 }
 
 
 void Thread::Join() {
-  pthread_join(thread_handle_data()->thread_, NULL);
+  pthread_join(data_->thread_, NULL);
 }
 
 

src/platform-macos.cc

@@ -392,50 +392,14 @@ bool VirtualMemory::Uncommit(void* address, size_t size) {
 }
 
 
-class ThreadHandle::PlatformData : public Malloced {
+class Thread::PlatformData : public Malloced {
  public:
-  explicit PlatformData(ThreadHandle::Kind kind) {
-    Initialize(kind);
-  }
-
-  void Initialize(ThreadHandle::Kind kind) {
-    switch (kind) {
-      case ThreadHandle::SELF: thread_ = pthread_self(); break;
-      case ThreadHandle::INVALID: thread_ = kNoThread; break;
-    }
-  }
+  PlatformData() : thread_(kNoThread) {}
   pthread_t thread_;  // Thread handle for pthread.
 };
 
-
-
-ThreadHandle::ThreadHandle(Kind kind) {
-  data_ = new PlatformData(kind);
-}
-
-
-void ThreadHandle::Initialize(ThreadHandle::Kind kind) {
-  data_->Initialize(kind);
-}
-
-
-ThreadHandle::~ThreadHandle() {
-  delete data_;
-}
-
-
-bool ThreadHandle::IsSelf() const {
-  return pthread_equal(data_->thread_, pthread_self());
-}
-
-
-bool ThreadHandle::IsValid() const {
-  return data_->thread_ != kNoThread;
-}
-
-
 Thread::Thread(Isolate* isolate, const Options& options)
-    : ThreadHandle(ThreadHandle::INVALID),
+    : data_(new PlatformData),
       isolate_(isolate),
       stack_size_(options.stack_size) {
   set_name(options.name);
@@ -443,7 +407,7 @@ Thread::Thread(Isolate* isolate, const Options& options)
 
 
 Thread::Thread(Isolate* isolate, const char* name)
-    : ThreadHandle(ThreadHandle::INVALID),
+    : data_(new PlatformData),
       isolate_(isolate),
       stack_size_(0) {
   set_name(name);
@@ -451,6 +415,7 @@ Thread::Thread(Isolate* isolate, const char* name)
 
 
 Thread::~Thread() {
+  delete data_;
 }
 
 
@@ -476,9 +441,9 @@ static void* ThreadEntry(void* arg) {
   // This is also initialized by the first argument to pthread_create() but we
   // don't know which thread will run first (the original thread or the new
   // one) so we initialize it here too.
-  thread->thread_handle_data()->thread_ = pthread_self();
+  thread->data()->thread_ = pthread_self();
   SetThreadName(thread->name());
-  ASSERT(thread->IsValid());
+  ASSERT(thread->data()->thread_ != kNoThread);
   Thread::SetThreadLocal(Isolate::isolate_key(), thread->isolate());
   thread->Run();
   return NULL;
@@ -499,13 +464,13 @@ void Thread::Start() {
     pthread_attr_setstacksize(&attr, static_cast<size_t>(stack_size_));
     attr_ptr = &attr;
   }
-  pthread_create(&thread_handle_data()->thread_, attr_ptr, ThreadEntry, this);
-  ASSERT(IsValid());
+  pthread_create(&data_->thread_, attr_ptr, ThreadEntry, this);
+  ASSERT(data_->thread_ != kNoThread);
 }
 
 
 void Thread::Join() {
-  pthread_join(thread_handle_data()->thread_, NULL);
+  pthread_join(data_->thread_, NULL);
 }
 
 

src/platform-nullos.cc

@@ -299,9 +299,9 @@ bool VirtualMemory::Uncommit(void* address, size_t size) {
 }
 
 
-class ThreadHandle::PlatformData : public Malloced {
+class Thread::PlatformData : public Malloced {
  public:
-  explicit PlatformData(ThreadHandle::Kind kind) {
+  PlatformData() {
     UNIMPLEMENTED();
   }
 
@@ -309,39 +309,8 @@ class ThreadHandle::PlatformData : public Malloced {
 };
 
 
-ThreadHandle::ThreadHandle(Kind kind) {
-  UNIMPLEMENTED();
-  // Shared setup follows.
-  data_ = new PlatformData(kind);
-}
-
-
-void ThreadHandle::Initialize(ThreadHandle::Kind kind) {
-  UNIMPLEMENTED();
-}
-
-
-ThreadHandle::~ThreadHandle() {
-  UNIMPLEMENTED();
-  // Shared tear down follows.
-  delete data_;
-}
-
-
-bool ThreadHandle::IsSelf() const {
-  UNIMPLEMENTED();
-  return false;
-}
-
-
-bool ThreadHandle::IsValid() const {
-  UNIMPLEMENTED();
-  return false;
-}
-
-
 Thread::Thread(Isolate* isolate, const Options& options)
-    : ThreadHandle(ThreadHandle::INVALID),
+    : data_(new PlatformData()),
       isolate_(isolate),
       stack_size_(options.stack_size) {
   set_name(options.name);
@@ -350,7 +319,7 @@ Thread::Thread(Isolate* isolate, const Options& options)
 
 
 Thread::Thread(Isolate* isolate, const char* name)
-    : ThreadHandle(ThreadHandle::INVALID),
+    : data_(new PlatformData()),
       isolate_(isolate),
       stack_size_(0) {
   set_name(name);
@@ -359,6 +328,7 @@ Thread::Thread(Isolate* isolate, const char* name)
 
 
 Thread::~Thread() {
+  delete data_;
   UNIMPLEMENTED();
 }
 

src/platform-openbsd.cc

@@ -359,49 +359,16 @@ bool VirtualMemory::Uncommit(void* address, size_t size) {
 }
 
 
-class ThreadHandle::PlatformData : public Malloced {
+class Thread::PlatformData : public Malloced {
  public:
-  explicit PlatformData(ThreadHandle::Kind kind) {
-    Initialize(kind);
-  }
+  PlatformData() : thread_(kNoThread) {}
 
-  void Initialize(ThreadHandle::Kind kind) {
-    switch (kind) {
-      case ThreadHandle::SELF: thread_ = pthread_self(); break;
-      case ThreadHandle::INVALID: thread_ = kNoThread; break;
-    }
-  }
   pthread_t thread_;  // Thread handle for pthread.
 };
 
 
-ThreadHandle::ThreadHandle(Kind kind) {
-  data_ = new PlatformData(kind);
-}
-
-
-void ThreadHandle::Initialize(ThreadHandle::Kind kind) {
-  data_->Initialize(kind);
-}
-
-
-ThreadHandle::~ThreadHandle() {
-  delete data_;
-}
-
-
-bool ThreadHandle::IsSelf() const {
-  return pthread_equal(data_->thread_, pthread_self());
-}
-
-
-bool ThreadHandle::IsValid() const {
-  return data_->thread_ != kNoThread;
-}
-
-
 Thread::Thread(Isolate* isolate, const Options& options)
-    : ThreadHandle(ThreadHandle::INVALID),
+    : data_(new PlatformData()),
       isolate_(isolate),
       stack_size_(options.stack_size) {
   set_name(options.name);
@@ -409,7 +376,7 @@ Thread::Thread(Isolate* isolate, const Options& options)
 
 
 Thread::Thread(Isolate* isolate, const char* name)
-    : ThreadHandle(ThreadHandle::INVALID),
+    : data_(new PlatfromData()),
       isolate_(isolate),
       stack_size_(0) {
   set_name(name);
@@ -417,6 +384,7 @@ Thread::Thread(Isolate* isolate, const char* name)
 
 
 Thread::~Thread() {
+  delete data_;
 }
 
 
@@ -425,8 +393,8 @@ static void* ThreadEntry(void* arg) {
   // This is also initialized by the first argument to pthread_create() but we
   // don't know which thread will run first (the original thread or the new
   // one) so we initialize it here too.
-  thread->thread_handle_data()->thread_ = pthread_self();
-  ASSERT(thread->IsValid());
+  thread->data()->thread_ = pthread_self();
+  ASSERT(thread->data()->thread_ != kNoThread);
   Thread::SetThreadLocal(Isolate::isolate_key(), thread->isolate());
   thread->Run();
   return NULL;
@@ -447,13 +415,13 @@ void Thread::Start() {
     pthread_attr_setstacksize(&attr, static_cast<size_t>(stack_size_));
     attr_ptr = &attr;
   }
-  pthread_create(&thread_handle_data()->thread_, attr_ptr, ThreadEntry, this);
+  pthread_create(&data_->thread_, attr_ptr, ThreadEntry, this);
   ASSERT(IsValid());
 }
 
 
 void Thread::Join() {
-  pthread_join(thread_handle_data()->thread_, NULL);
+  pthread_join(data_->thread_, NULL);
 }
 
 

src/platform-solaris.cc

@@ -373,50 +373,15 @@ bool VirtualMemory::Uncommit(void* address, size_t size) {
 }
 
 
-class ThreadHandle::PlatformData : public Malloced {
+class Thread::PlatformData : public Malloced {
  public:
-  explicit PlatformData(ThreadHandle::Kind kind) {
-    Initialize(kind);
-  }
-
-  void Initialize(ThreadHandle::Kind kind) {
-    switch (kind) {
-      case ThreadHandle::SELF: thread_ = pthread_self(); break;
-      case ThreadHandle::INVALID: thread_ = kNoThread; break;
-    }
-  }
+  PlatformData() : thread_(kNoThread) {  }
 
   pthread_t thread_;  // Thread handle for pthread.
 };
 
-
-ThreadHandle::ThreadHandle(Kind kind) {
-  data_ = new PlatformData(kind);
-}
-
-
-void ThreadHandle::Initialize(ThreadHandle::Kind kind) {
-  data_->Initialize(kind);
-}
-
-
-ThreadHandle::~ThreadHandle() {
-  delete data_;
-}
-
-
-bool ThreadHandle::IsSelf() const {
-  return pthread_equal(data_->thread_, pthread_self());
-}
-
-
-bool ThreadHandle::IsValid() const {
-  return data_->thread_ != kNoThread;
-}
-
-
 Thread::Thread(Isolate* isolate, const Options& options)
-    : ThreadHandle(ThreadHandle::INVALID),
+    : data_(new PlatformData()),
       isolate_(isolate),
       stack_size_(options.stack_size) {
   set_name(options.name);
@@ -424,7 +389,7 @@ Thread::Thread(Isolate* isolate, const Options& options)
 
 
 Thread::Thread(Isolate* isolate, const char* name)
-    : ThreadHandle(ThreadHandle::INVALID),
+    : data_(new PlatformData()),
       isolate_(isolate),
       stack_size_(0) {
   set_name(name);
@@ -432,6 +397,7 @@ Thread::Thread(Isolate* isolate, const char* name)
 
 
 Thread::~Thread() {
+  delete data_;
 }
 
 
@@ -440,8 +406,8 @@ static void* ThreadEntry(void* arg) {
   // This is also initialized by the first argument to pthread_create() but we
   // don't know which thread will run first (the original thread or the new
   // one) so we initialize it here too.
-  thread->thread_handle_data()->thread_ = pthread_self();
-  ASSERT(thread->IsValid());
+  thread->data()->thread_ = pthread_self();
+  ASSERT(thread->data()->thread_ != kNoThread);
   Thread::SetThreadLocal(Isolate::isolate_key(), thread->isolate());
   thread->Run();
   return NULL;
@@ -462,13 +428,13 @@ void Thread::Start() {
     pthread_attr_setstacksize(&attr, static_cast<size_t>(stack_size_));
     attr_ptr = &attr;
   }
-  pthread_create(&thread_handle_data()->thread_, NULL, ThreadEntry, this);
-  ASSERT(IsValid());
+  pthread_create(&data_->thread_, NULL, ThreadEntry, this);
+  ASSERT(data_->thread_ != kNoThread);
 }
 
 
 void Thread::Join() {
-  pthread_join(thread_handle_data()->thread_, NULL);
+  pthread_join(data_->thread_, NULL);
 }
 
 

src/platform-win32.cc

@@ -1496,41 +1496,12 @@ static unsigned int __stdcall ThreadEntry(void* arg) {
   // This is also initialized by the last parameter to _beginthreadex() but we
   // don't know which thread will run first (the original thread or the new
   // one) so we initialize it here too.
-  thread->thread_handle_data()->tid_ = GetCurrentThreadId();
   Thread::SetThreadLocal(Isolate::isolate_key(), thread->isolate());
   thread->Run();
   return 0;
 }
 
 
-// Initialize thread handle to invalid handle.
-ThreadHandle::ThreadHandle(ThreadHandle::Kind kind) {
-  data_ = new PlatformData(kind);
-}
-
-
-ThreadHandle::~ThreadHandle() {
-  delete data_;
-}
-
-
-// The thread is running if it has the same id as the current thread.
-bool ThreadHandle::IsSelf() const {
-  return GetCurrentThreadId() == data_->tid_;
-}
-
-
-// Test for invalid thread handle.
-bool ThreadHandle::IsValid() const {
-  return data_->tid_ != kNoThreadId;
-}
-
-
-void ThreadHandle::Initialize(ThreadHandle::Kind kind) {
-  data_->Initialize(kind);
-}
-
-
 class Thread::PlatformData : public Malloced {
  public:
   explicit PlatformData(HANDLE thread) : thread_(thread) {}
@@ -1542,8 +1513,7 @@ class Thread::PlatformData : public Malloced {
 // handle until it is started.
 
 Thread::Thread(Isolate* isolate, const Options& options)
-    : ThreadHandle(ThreadHandle::INVALID),
-      isolate_(isolate),
+    : isolate_(isolate),
       stack_size_(options.stack_size) {
   data_ = new PlatformData(kNoThread);
   set_name(options.name);
@@ -1551,8 +1521,7 @@ Thread::Thread(Isolate* isolate, const Options& options)
 
 
 Thread::Thread(Isolate* isolate, const char* name)
-    : ThreadHandle(ThreadHandle::INVALID),
-      isolate_(isolate),
+    : isolate_(isolate),
       stack_size_(0) {
   data_ = new PlatformData(kNoThread);
   set_name(name);

src/platform.h

@@ -354,40 +354,6 @@ class VirtualMemory {
   size_t size_;  // Size of the virtual memory.
 };
 
-
-// ----------------------------------------------------------------------------
-// ThreadHandle
-//
-// A ThreadHandle represents a thread identifier for a thread. The ThreadHandle
-// does not own the underlying os handle. Thread handles can be used for
-// refering to threads and testing equality.
-
-class ThreadHandle {
- public:
-  enum Kind { SELF, INVALID };
-  explicit ThreadHandle(Kind kind);
-
-  // Destructor.
-  ~ThreadHandle();
-
-  // Test for thread running.
-  bool IsSelf() const;
-
-  // Test for valid thread handle.
-  bool IsValid() const;
-
-  // Get platform-specific data.
-  class PlatformData;
-  PlatformData* thread_handle_data() { return data_; }
-
-  // Initialize the handle to kind
-  void Initialize(Kind kind);
-
- private:
-  PlatformData* data_;  // Captures platform dependent data.
-};
-
-
 // ----------------------------------------------------------------------------
 // Thread
 //
@@ -396,7 +362,7 @@ class ThreadHandle {
 // thread. The Thread object should not be deallocated before the thread has
 // terminated.
 
-class Thread: public ThreadHandle {
+class Thread {
  public:
   // Opaque data type for thread-local storage keys.
   // LOCAL_STORAGE_KEY_MIN_VALUE and LOCAL_STORAGE_KEY_MAX_VALUE are specified
@@ -468,11 +434,15 @@ class Thread: public ThreadHandle {
   // The thread name length is limited to 16 based on Linux's implementation of
   // prctl().
   static const int kMaxThreadNameLength = 16;
+
+  class PlatformData;
+  PlatformData* data() { return data_; }
+
  private:
   void set_name(const char *name);
 
-  class PlatformData;
   PlatformData* data_;
+
   Isolate* isolate_;
   char name_[kMaxThreadNameLength];
   int stack_size_;

src/runtime.cc

@@ -10171,8 +10171,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetThreadDetails) {
     details->set(kThreadDetailsCurrentThreadIndex,
                  isolate->heap()->true_value());
     details->set(kThreadDetailsThreadIdIndex,
-                 Smi::FromInt(
-                     isolate->thread_manager()->CurrentId()));
+                 Smi::FromInt(ThreadId::Current().ToInteger()));
   } else {
     // Find the thread with the requested index.
     int n = 1;
@@ -10189,7 +10188,8 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetThreadDetails) {
     // Fill the details.
     details->set(kThreadDetailsCurrentThreadIndex,
                  isolate->heap()->false_value());
-    details->set(kThreadDetailsThreadIdIndex, Smi::FromInt(thread->id()));
+    details->set(kThreadDetailsThreadIdIndex,
+                 Smi::FromInt(thread->id().ToInteger()));
   }
 
   // Convert to JS array and return.

src/top.cc

@@ -69,8 +69,7 @@ void ThreadLocalTop::Initialize() {
 #endif
   try_catch_handler_address_ = NULL;
   context_ = NULL;
-  int id = Isolate::Current()->thread_manager()->CurrentId();
-  thread_id_ = (id == 0) ? ThreadManager::kInvalidId : id;
+  thread_id_ = ThreadId::Current();
   external_caught_exception_ = false;
   failed_access_check_callback_ = NULL;
   save_context_ = NULL;

src/v8.cc

@@ -63,8 +63,8 @@ bool V8::Initialize(Deserializer* des) {
   }
 
   ASSERT(i::Isolate::CurrentPerIsolateThreadData() != NULL);
-  ASSERT(i::Isolate::CurrentPerIsolateThreadData()->thread_id() ==
-         i::Thread::GetThreadLocalInt(i::Isolate::thread_id_key()));
+  ASSERT(i::Isolate::CurrentPerIsolateThreadData()->thread_id().Equals(
+           i::ThreadId::Current()));
   ASSERT(i::Isolate::CurrentPerIsolateThreadData()->isolate() ==
          i::Isolate::Current());
 

src/v8threads.cc

@@ -147,11 +147,11 @@ bool ThreadManager::RestoreThread() {
   // First check whether the current thread has been 'lazily archived', ie
   // not archived at all.  If that is the case we put the state storage we
   // had prepared back in the free list, since we didn't need it after all.
-  if (lazily_archived_thread_.IsSelf()) {
-    lazily_archived_thread_.Initialize(ThreadHandle::INVALID);
+  if (lazily_archived_thread_.Equals(ThreadId::Current())) {
+    lazily_archived_thread_ = ThreadId::Invalid();
     ASSERT(Isolate::CurrentPerIsolateThreadData()->thread_state() ==
            lazily_archived_thread_state_);
-    lazily_archived_thread_state_->set_id(kInvalidId);
+    lazily_archived_thread_state_->set_id(ThreadId::Invalid());
     lazily_archived_thread_state_->LinkInto(ThreadState::FREE_LIST);
     lazily_archived_thread_state_ = NULL;
     Isolate::CurrentPerIsolateThreadData()->set_thread_state(NULL);
@@ -190,7 +190,7 @@ bool ThreadManager::RestoreThread() {
     isolate_->stack_guard()->TerminateExecution();
     state->set_terminate_on_restore(false);
   }
-  state->set_id(kInvalidId);
+  state->set_id(ThreadId::Invalid());
   state->Unlink();
   state->LinkInto(ThreadState::FREE_LIST);
   return true;
@@ -199,13 +199,13 @@ bool ThreadManager::RestoreThread() {
 
 void ThreadManager::Lock() {
   mutex_->Lock();
-  mutex_owner_.Initialize(ThreadHandle::SELF);
+  mutex_owner_ = ThreadId::Current();
   ASSERT(IsLockedByCurrentThread());
 }
 
 
 void ThreadManager::Unlock() {
-  mutex_owner_.Initialize(ThreadHandle::INVALID);
+  mutex_owner_ = ThreadId::Invalid();
   mutex_->Unlock();
 }
 
@@ -224,7 +224,7 @@ static int ArchiveSpacePerThread() {
 
 
 ThreadState::ThreadState(ThreadManager* thread_manager)
-    : id_(ThreadManager::kInvalidId),
+    : id_(ThreadId::Invalid()),
       terminate_on_restore_(false),
       next_(this),
       previous_(this),
@@ -282,8 +282,8 @@ ThreadState* ThreadState::Next() {
 // defined as 0.)
 ThreadManager::ThreadManager()
     : mutex_(OS::CreateMutex()),
-      mutex_owner_(ThreadHandle::INVALID),
-      lazily_archived_thread_(ThreadHandle::INVALID),
+      mutex_owner_(ThreadId::Invalid()),
+      lazily_archived_thread_(ThreadId::Invalid()),
       lazily_archived_thread_state_(NULL),
       free_anchor_(NULL),
       in_use_anchor_(NULL) {
@@ -298,16 +298,16 @@ ThreadManager::~ThreadManager() {
 
 
 void ThreadManager::ArchiveThread() {
-  ASSERT(!lazily_archived_thread_.IsValid());
+  ASSERT(lazily_archived_thread_.Equals(ThreadId::Invalid()));
   ASSERT(!IsArchived());
   ThreadState* state = GetFreeThreadState();
   state->Unlink();
   Isolate::CurrentPerIsolateThreadData()->set_thread_state(state);
-  lazily_archived_thread_.Initialize(ThreadHandle::SELF);
+  lazily_archived_thread_ = ThreadId::Current();
   lazily_archived_thread_state_ = state;
-  ASSERT(state->id() == kInvalidId);
+  ASSERT(state->id().Equals(ThreadId::Invalid()));
   state->set_id(CurrentId());
-  ASSERT(state->id() != kInvalidId);
+  ASSERT(!state->id().Equals(ThreadId::Invalid()));
 }
 
 
@@ -326,7 +326,7 @@ void ThreadManager::EagerlyArchiveThread() {
   to = isolate_->stack_guard()->ArchiveStackGuard(to);
   to = isolate_->regexp_stack()->ArchiveStack(to);
   to = isolate_->bootstrapper()->ArchiveState(to);
-  lazily_archived_thread_.Initialize(ThreadHandle::INVALID);
+  lazily_archived_thread_ = ThreadId::Invalid();
   lazily_archived_thread_state_ = NULL;
 }
 
@@ -373,16 +373,16 @@ void ThreadManager::IterateArchivedThreads(ThreadVisitor* v) {
 }
 
 
-int ThreadManager::CurrentId() {
-  return Thread::GetThreadLocalInt(Isolate::thread_id_key());
+ThreadId ThreadManager::CurrentId() {
+  return ThreadId::Current();
 }
 
 
-void ThreadManager::TerminateExecution(int thread_id) {
+void ThreadManager::TerminateExecution(ThreadId thread_id) {
   for (ThreadState* state = FirstThreadStateInUse();
        state != NULL;
        state = state->Next()) {
-    if (thread_id == state->id()) {
+    if (thread_id.Equals(state->id())) {
       state->set_terminate_on_restore(true);
     }
   }

src/v8threads.h

@@ -43,8 +43,8 @@ class ThreadState {
   void Unlink();
 
   // Id of thread.
-  void set_id(int id) { id_ = id; }
-  int id() { return id_; }
+  void set_id(ThreadId id) { id_ = id; }
+  ThreadId id() { return id_; }
 
   // Should the thread be terminated when it is restored?
   bool terminate_on_restore() { return terminate_on_restore_; }
@@ -59,7 +59,7 @@ class ThreadState {
 
   void AllocateSpace();
 
-  int id_;
+  ThreadId id_;
   bool terminate_on_restore_;
   char* data_;
   ThreadState* next_;
@@ -97,17 +97,18 @@ class ThreadManager {
 
   void Iterate(ObjectVisitor* v);
   void IterateArchivedThreads(ThreadVisitor* v);
-  bool IsLockedByCurrentThread() { return mutex_owner_.IsSelf(); }
+  bool IsLockedByCurrentThread() {
+    return mutex_owner_.Equals(ThreadId::Current());
+  }
 
-  int CurrentId();
+  ThreadId CurrentId();
 
-  void TerminateExecution(int thread_id);
+  void TerminateExecution(ThreadId thread_id);
 
   // Iterate over in-use states.
   ThreadState* FirstThreadStateInUse();
   ThreadState* GetFreeThreadState();
 
-  static const int kInvalidId = -1;
  private:
   ThreadManager();
   ~ThreadManager();
@@ -115,8 +116,8 @@ class ThreadManager {
   void EagerlyArchiveThread();
 
   Mutex* mutex_;
-  ThreadHandle mutex_owner_;
-  ThreadHandle lazily_archived_thread_;
+  ThreadId mutex_owner_;
+  ThreadId lazily_archived_thread_;
   ThreadState* lazily_archived_thread_state_;
 
   // In the following two lists there is always at least one object on the list.

test/cctest/test-threads.cc

@@ -28,6 +28,7 @@
 #include "v8.h"
 
 #include "platform.h"
+#include "isolate.h"
 
 #include "cctest.h"
 
@@ -136,3 +137,55 @@ TEST(JSFunctionResultCachesInTwoThreads) {
 
   CHECK_EQ(DONE, turn);
 }
+
+class ThreadIdValidationThread : public v8::internal::Thread {
+ public:
+  ThreadIdValidationThread(i::Thread* thread_to_start,
+                           i::List<i::ThreadId>* refs,
+                           unsigned int thread_no,
+                           i::Semaphore* semaphore)
+    : Thread(NULL, "ThreadRefValidationThread"),
+      refs_(refs), thread_no_(thread_no), thread_to_start_(thread_to_start),
+      semaphore_(semaphore) {
+  }
+
+  void Run() {
+    i::ThreadId thread_id = i::ThreadId::Current();
+    for (int i = 0; i < thread_no_; i++) {
+      CHECK(!(*refs_)[i].Equals(thread_id));
+    }
+    CHECK(thread_id.IsValid());
+    (*refs_)[thread_no_] = thread_id;
+    if (thread_to_start_ != NULL) {
+      thread_to_start_->Start();
+    }
+    semaphore_->Signal();
+  }
+ private:
+  i::List<i::ThreadId>* refs_;
+  int thread_no_;
+  i::Thread* thread_to_start_;
+  i::Semaphore* semaphore_;
+};
+
+TEST(ThreadIdValidation) {
+  const int kNThreads = 100;
+  i::List<ThreadIdValidationThread*> threads(kNThreads);
+  i::List<i::ThreadId> refs(kNThreads);
+  i::Semaphore* semaphore = i::OS::CreateSemaphore(0);
+  ThreadIdValidationThread* prev = NULL;
+  for (int i = kNThreads - 1; i >= 0; i--) {
+    ThreadIdValidationThread* newThread =
+        new ThreadIdValidationThread(prev, &refs, i, semaphore);
+    threads.Add(newThread);
+    prev = newThread;
+    refs.Add(i::ThreadId::Invalid());
+  }
+  prev->Start();
+  for (int i = 0; i < kNThreads; i++) {
+    semaphore->Wait();
+  }
+  for (int i = 0; i < kNThreads; i++) {
+    delete threads[i];
+  }
+}