Use PlatformThreadId, not int when dealing with thread ids.

Windows uses a DWORD (unsigned long) for thread ids and POSIX uses a
pid_t (int on Linux) for the same. In the code, we are currently
stuffing thread ids into an int which is dangerous on Windows (because
DWORDS can exceed an int and wrap) and will break if pid_t is ever !=
int.

This change changes all the places where we currently have an int to
use a new typedef, PlatformThreadId. This change also needs to occur
for process ids, but I'm not doing that in this CL.

Review URL: http://codereview.chromium.org/18677


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

11 files changed, 37 insertions, 16 deletions

diff --git a/base/message_pump_glib.cc b/base/message_pump_glib.cc
index 2ff3cc0..602420e 100644
--- a/base/message_pump_glib.cc
+++ b/base/message_pump_glib.cc
@@ -122,7 +122,7 @@ void MessagePumpForUI::Run(Delegate* delegate) {
 #ifndef NDEBUG
   // Make sure we only run this on one thread.  GTK only has one message pump
   // so we can only have one UI loop per process.
-  static int thread_id = PlatformThread::CurrentId();
+  static PlatformThreadId thread_id = PlatformThread::CurrentId();
   DCHECK(thread_id == PlatformThread::CurrentId()) <<
       "Running MessagePumpForUI on two different threads; "
       "this is unsupported by GLib!";
diff --git a/base/non_thread_safe.h b/base/non_thread_safe.h
index 45a352d..2791fe0 100644
--- a/base/non_thread_safe.h
+++ b/base/non_thread_safe.h
@@ -6,6 +6,7 @@
 #define BASE_NON_THREAD_SAFE_H__
 
 #include "base/logging.h"
+#include "base/platform_thread.h"
 
 // A helper class used to help verify that methods of a class are
 // called from the same thread.  One can inherit from this class and use
@@ -35,7 +36,7 @@ class NonThreadSafe {
   bool CalledOnValidThread() const;
 
  private:
-  int valid_thread_id_;
+  PlatformThreadId valid_thread_id_;
 };
 #else
 // Do nothing in release mode.
diff --git a/base/platform_thread.h b/base/platform_thread.h
index a999e8d..1ae9c0f 100644
--- a/base/platform_thread.h
+++ b/base/platform_thread.h
@@ -16,17 +16,26 @@
 // should not initialize it to a value, like 0.  If it's a member variable, the
 // constructor can safely "value initialize" using () in the initializer list.
 #if defined(OS_WIN)
+#include <windows.h>
+typedef DWORD PlatformThreadId;
 typedef void* PlatformThreadHandle;  // HANDLE
 #elif defined(OS_POSIX)
 #include <pthread.h>
 typedef pthread_t PlatformThreadHandle;
+#if defined(OS_LINUX)
+#include <unistd.h>
+typedef pid_t PlatformThreadId;
+#elif defined(OS_MACOSX)
+#include <mach/mach.h>
+typedef mach_port_t PlatformThreadId;
+#endif
 #endif
 
 // A namespace for low-level thread functions.
 class PlatformThread {
  public:
   // Gets the current thread id, which may be useful for logging purposes.
-  static int CurrentId();
+  static PlatformThreadId CurrentId();
 
   // Yield the current thread so another thread can be scheduled.
   static void YieldCurrentThread();
diff --git a/base/platform_thread_posix.cc b/base/platform_thread_posix.cc
index 77f5bee..4cca4e6 100644
--- a/base/platform_thread_posix.cc
+++ b/base/platform_thread_posix.cc
@@ -28,7 +28,7 @@ static void* ThreadFunc(void* closure) {
 }
 
 // static
-int PlatformThread::CurrentId() {
+PlatformThreadId PlatformThread::CurrentId() {
   // Pthreads doesn't have the concept of a thread ID, so we have to reach down
   // into the kernel.
 #if defined(OS_MACOSX)
diff --git a/base/platform_thread_win.cc b/base/platform_thread_win.cc
index c390e18..d813bb72 100644
--- a/base/platform_thread_win.cc
+++ b/base/platform_thread_win.cc
@@ -30,7 +30,7 @@ DWORD __stdcall ThreadFunc(void* closure) {
 }  // namespace
 
 // static
-int PlatformThread::CurrentId() {
+PlatformThreadId PlatformThread::CurrentId() {
   return GetCurrentThreadId();
 }
 
diff --git a/base/simple_thread.h b/base/simple_thread.h
index edcbfbe..17a2f6d 100644
--- a/base/simple_thread.h
+++ b/base/simple_thread.h
@@ -98,7 +98,7 @@ class SimpleThread : public PlatformThread::Delegate {
   std::string name() { return name_; }
 
   // Return the thread id, only valid after Start().
-  int tid() { return tid_; }
+  PlatformThreadId tid() { return tid_; }
 
   // Return True if Start() has ever been called.
   bool HasBeenStarted() { return event_.IsSignaled(); }
@@ -112,7 +112,7 @@ class SimpleThread : public PlatformThread::Delegate {
   const Options options_;
   PlatformThreadHandle thread_;  // PlatformThread handle, invalid after Join!
   WaitableEvent event_;          // Signaled if Start() was ever called.
-  int tid_;                      // The backing thread's id.
+  PlatformThreadId tid_;         // The backing thread's id.
   bool joined_;                  // True if Join has been called.
 };
 
diff --git a/base/stats_table.cc b/base/stats_table.cc
index 1f25457..31570b5 100644
--- a/base/stats_table.cc
+++ b/base/stats_table.cc
@@ -126,7 +126,7 @@ class StatsTablePrivate {
     return &thread_names_table_[
       (slot_id-1) * (StatsTable::kMaxThreadNameLength)];
   }
-  int* thread_tid(int slot_id) const {
+  PlatformThreadId* thread_tid(int slot_id) const {
     return &(thread_tid_table_[slot_id-1]);
   }
   int* thread_pid(int slot_id) const {
@@ -155,7 +155,7 @@ class StatsTablePrivate {
   base::SharedMemory shared_memory_;
   TableHeader* table_header_;
   char* thread_names_table_;
-  int* thread_tid_table_;
+  PlatformThreadId* thread_tid_table_;
   int* thread_pid_table_;
   char* counter_names_table_;
   int* data_table_;
@@ -217,7 +217,7 @@ void StatsTablePrivate::ComputeMappedPointers(void* memory) {
             max_threads() * StatsTable::kMaxThreadNameLength;
   offset += AlignOffset(offset);
 
-  thread_tid_table_ = reinterpret_cast<int*>(data + offset);
+  thread_tid_table_ = reinterpret_cast<PlatformThreadId*>(data + offset);
   offset += sizeof(int) * max_threads();
   offset += AlignOffset(offset);
 
diff --git a/base/thread.h b/base/thread.h
index fd7fea2..13aa35b 100644
--- a/base/thread.h
+++ b/base/thread.h
@@ -102,7 +102,7 @@ class Thread : PlatformThread::Delegate {
   PlatformThreadHandle thread_handle() { return thread_; }
 
   // The thread ID.
-  int thread_id() const { return thread_id_; }
+  PlatformThreadId thread_id() const { return thread_id_; }
 
  protected:
   // Called just prior to starting the message loop
@@ -134,7 +134,7 @@ class Thread : PlatformThread::Delegate {
   MessageLoop* message_loop_;
 
   // Our thread's ID.
-  int thread_id_;
+  PlatformThreadId thread_id_;
 
   // The name of the thread.  Used for debugging purposes.
   std::string name_;
diff --git a/base/thread_collision_warner.cc b/base/thread_collision_warner.cc
index 1c51a3e..03245e1 100644
--- a/base/thread_collision_warner.cc
+++ b/base/thread_collision_warner.cc
@@ -13,11 +13,22 @@ void DCheckAsserter::warn() {
   NOTREACHED() << "Thread Collision";
 }
 
+static subtle::Atomic32 CurrentThread() {
+  const PlatformThreadId current_thread_id = PlatformThread::CurrentId();
+  // We need to get the thread id into an atomic data type. This might be a
+  // truncating conversion, but any loss-of-information just increases the
+  // chance of a fault negative, not a false positive.
+  const subtle::Atomic32 atomic_thread_id =
+      static_cast<subtle::Atomic32>(current_thread_id);
+
+  return atomic_thread_id;
+}
+
 void ThreadCollisionWarner::EnterSelf() {
   // If the active thread is 0 then I'll write the current thread ID
   // if two or more threads arrive here only one will succeed to
   // write on valid_thread_id_ the current thread ID.
-  const int current_thread_id = PlatformThread::CurrentId();
+  subtle::Atomic32 current_thread_id = CurrentThread();
 
   int previous_value = subtle::NoBarrier_CompareAndSwap(&valid_thread_id_,
                                                         0,
@@ -32,7 +43,7 @@ void ThreadCollisionWarner::EnterSelf() {
 }
 
 void ThreadCollisionWarner::Enter() {
-  const int current_thread_id = PlatformThread::CurrentId();
+  subtle::Atomic32 current_thread_id = CurrentThread();
 
   if (subtle::NoBarrier_CompareAndSwap(&valid_thread_id_,
                                        0,
diff --git a/base/thread_collision_warner.h b/base/thread_collision_warner.h
index fa8b7ee..6545451 100644
--- a/base/thread_collision_warner.h
+++ b/base/thread_collision_warner.h
@@ -225,7 +225,7 @@ class ThreadCollisionWarner {
 
   // This stores the thread id that is inside the critical section, if the 
   // value is 0 then no thread is inside.
-  volatile int valid_thread_id_;
+  volatile subtle::Atomic32 valid_thread_id_;
 
   // Counter to trace how many time a critical section was "pinned"
   // (when allowed) in order to unpin it when counter_ reaches 0.
diff --git a/net/url_request/url_request_job_manager.h b/net/url_request/url_request_job_manager.h
index d5924a0..dab3333 100644
--- a/net/url_request/url_request_job_manager.h
+++ b/net/url_request/url_request_job_manager.h
@@ -57,7 +57,7 @@ class URLRequestJobManager {
 #ifndef NDEBUG
   // We use this to assert that CreateJob and the registration functions all
   // run on the same thread.
-  mutable int allowed_thread_;
+  mutable PlatformThreadId allowed_thread_;
   mutable bool allowed_thread_initialized_;
 
   // The first guy to call this function sets the allowed thread.  This way we