Simplifies the threading implementation and improves some comments.

include/gtest/internal/gtest-port.h

@@ -733,6 +733,16 @@ inline void FlushInfoLog() { fflush(NULL); }
     else \
       GTEST_LOG_(FATAL) << "Condition " #condition " failed. "
 
+// An all-mode assert to verify that the given POSIX-style function
+// call returns 0 (indicating success).  Known limitation: this
+// doesn't expand to a balanced 'if' statement, so enclose the macro
+// in {} if you need to use it as the only statement in an 'if'
+// branch.
+#define GTEST_CHECK_POSIX_SUCCESS_(posix_call) \
+  if (const int gtest_error = (posix_call)) \
+    GTEST_LOG_(FATAL) << #posix_call << "failed with error " \
+                      << gtest_error
+
 #if GTEST_HAS_STREAM_REDIRECTION_
 
 // Defines the stderr capturer:
@@ -776,54 +786,75 @@ const ::std::vector<String>& GetArgvs();
 //                            // of the current scope.
 //
 // MutexBase implements behavior for both statically and dynamically
-// allocated mutexes.  Do not use the MutexBase type directly.  Instead,
+// allocated mutexes.  Do not use MutexBase directly.  Instead, write
-// define a static mutex using the GTEST_DEFINE_STATIC_MUTEX_ macro:
+// the following to define a static mutex:
 //
 //   GTEST_DEFINE_STATIC_MUTEX_(g_some_mutex);
 //
-// Such mutex may also be forward-declared:
+// You can forward declare a static mutex like this:
 //
 //   GTEST_DECLARE_STATIC_MUTEX_(g_some_mutex);
 //
-// Do not use MutexBase for dynamic mutexes either.  Use the Mutex class
+// To create a dynamic mutex, just define an object of type Mutex.
-// for them.
 class MutexBase {
  public:
-  void Lock();
+  // Acquires this mutex.
-  void Unlock();
+  void Lock() {
+    GTEST_CHECK_POSIX_SUCCESS_(pthread_mutex_lock(&mutex_));
+    owner_ = pthread_self();
+  }
+
+  // Releases this mutex.
+  void Unlock() {
+    // We don't protect writing to owner_ here, as it's the caller's
+    // responsibility to ensure that the current thread holds the
+    // mutex when this is called.
+    owner_ = 0;
+    GTEST_CHECK_POSIX_SUCCESS_(pthread_mutex_unlock(&mutex_));
+  }
 
   // Does nothing if the current thread holds the mutex. Otherwise, crashes
   // with high probability.
-  void AssertHeld() const;
+  void AssertHeld() const {
+    GTEST_CHECK_(owner_ == pthread_self())
+        << "The current thread is not holding the mutex @" << this;
+  }
 
- // We must be able to initialize objects of MutexBase used as static
+  // A static mutex may be used before main() is entered.  It may even
- // mutexes with initializer lists.  This means MutexBase has to be a POD.
+  // be used before the dynamic initialization stage.  Therefore we
- // The class members have to be public.
+  // must be able to initialize a static mutex object at link time.
+  // This means MutexBase has to be a POD and its member variables
+  // have to be public.
  public:
-  pthread_mutex_t mutex_;
+  pthread_mutex_t mutex_;  // The underlying pthread mutex.
-  pthread_t owner_;
+  pthread_t owner_;  // The thread holding the mutex; 0 means no one holds it.
 };
 
 // Forward-declares a static mutex.
 #define GTEST_DECLARE_STATIC_MUTEX_(mutex) \
     extern ::testing::internal::MutexBase mutex
 
-// Defines and statically initializes a static mutex.
+// Defines and statically (i.e. at link time) initializes a static mutex.
 #define GTEST_DEFINE_STATIC_MUTEX_(mutex) \
     ::testing::internal::MutexBase mutex = { PTHREAD_MUTEX_INITIALIZER, 0 }
 
-// The class Mutex supports only mutexes created at runtime. It shares its
+// The Mutex class can only be used for mutexes created at runtime. It
-// API with MutexBase otherwise.
+// shares its API with MutexBase otherwise.
 class Mutex : public MutexBase {
  public:
-  Mutex();
+  Mutex() {
-  ~Mutex();
+    GTEST_CHECK_POSIX_SUCCESS_(pthread_mutex_init(&mutex_, NULL));
+    owner_ = 0;
+  }
+  ~Mutex() {
+    GTEST_CHECK_POSIX_SUCCESS_(pthread_mutex_destroy(&mutex_));
+  }
 
  private:
   GTEST_DISALLOW_COPY_AND_ASSIGN_(Mutex);
 };
 
-// We cannot call it MutexLock directly as the ctor declaration would
+// We cannot name this class MutexLock as the ctor declaration would
 // conflict with a macro named MutexLock, which is defined on some
 // platforms.  Hence the typedef trick below.
 class GTestMutexLock {
@@ -844,39 +875,33 @@ typedef GTestMutexLock MutexLock;
 // Implements thread-local storage on pthreads-based systems.
 //
 //   // Thread 1
-// ThreadLocal<int> tl(100);
+//   ThreadLocal<int> tl(100);  // 100 is the default value for each thread.
 //
 //   // Thread 2
-// tl.set(150);
+//   tl.set(150);  // Changes the value for thread 2 only.
 //   EXPECT_EQ(150, tl.get());
 //
 //   // Thread 1
-// EXPECT_EQ(100, tl.get());  // On Thread 1, tl.get() returns original value.
+//   EXPECT_EQ(100, tl.get());  // In thread 1, tl has the original value.
 //   tl.set(200);
 //   EXPECT_EQ(200, tl.get());
 //
-// The default ThreadLocal constructor requires T to have a default
+// The template type argument T must have a public copy constructor.
-// constructor.  The single param constructor requires a copy contructor
+// In addition, the default ThreadLocal constructor requires T to have
-// from T.  A per-thread object managed by a ThreadLocal instance for a
+// a public default constructor.  An object managed by a ThreadLocal
-// thread is guaranteed to exist at least until the earliest of the two
+// instance for a thread is guaranteed to exist at least until the
-// events: (a) the thread terminates or (b) the ThreadLocal object
+// earliest of the two events: (a) the thread terminates or (b) the
-// managing it is destroyed.
+// ThreadLocal object is destroyed.
 template <typename T>
 class ThreadLocal {
  public:
-  ThreadLocal()
+  ThreadLocal() : key_(CreateKey()),
-      : key_(CreateKey()),
+                  default_() {}
-        default_(),
+  explicit ThreadLocal(const T& value) : key_(CreateKey()),
-        instance_creator_func_(DefaultConstructNewInstance) {}
+                                         default_(value) {}
-
-  explicit ThreadLocal(const T& value)
-      : key_(CreateKey()),
-        default_(value),
-        instance_creator_func_(CopyConstructNewInstance) {}
 
   ~ThreadLocal() {
-    const int err = pthread_key_delete(key_);
+    GTEST_CHECK_POSIX_SUCCESS_(pthread_key_delete(key_));
-    GTEST_CHECK_(err == 0) << "pthread_key_delete failed with error " << err;
   }
 
   T* pointer() { return GetOrCreateValue(); }
@@ -887,54 +912,43 @@ class ThreadLocal {
  private:
   static pthread_key_t CreateKey() {
     pthread_key_t key;
-    const int err = pthread_key_create(&key, &DeleteData);
+    GTEST_CHECK_POSIX_SUCCESS_(pthread_key_create(&key, &DeleteData));
-    GTEST_CHECK_(err == 0) << "pthread_key_create failed with error " << err;
     return key;
   }
 
   T* GetOrCreateValue() const {
-    T* value = static_cast<T*>(pthread_getspecific(key_));
+    T* const value = static_cast<T*>(pthread_getspecific(key_));
-    if (value == NULL) {
+    if (value != NULL)
-      value = (*instance_creator_func_)(default_);
-      const int err = pthread_setspecific(key_, value);
-      GTEST_CHECK_(err == 0) << "pthread_setspecific failed with error " << err;
-    }
       return value;
+
+    T* const new_value = new T(default_);
+    GTEST_CHECK_POSIX_SUCCESS_(pthread_setspecific(key_, new_value));
+    return new_value;
   }
 
   static void DeleteData(void* data) { delete static_cast<T*>(data); }
 
-  static T* DefaultConstructNewInstance(const T&) { return new T(); }
-
-  // Copy constructs new instance of T from default_.  Will not be
-  // instantiated unless this ThreadLocal is constructed by the single
-  // parameter constructor.
-  static T* CopyConstructNewInstance(const T& t) { return new T(t); }
-
   // A key pthreads uses for looking up per-thread values.
   const pthread_key_t key_;
-  // Contains the value that CopyConstructNewInstance copies from.
+  const T default_;  // The default value for each thread.
-  const T default_;
-  // Points to either DefaultConstructNewInstance or CopyConstructNewInstance.
-  T* (*const instance_creator_func_)(const T& default_);
 
   GTEST_DISALLOW_COPY_AND_ASSIGN_(ThreadLocal);
 };
 
-// Allows the controller thread pause execution of newly created test
+// Allows a controller thread to pause execution of newly created
-// threads until signalled. Instances of this class must be created and
+// threads until signalled.  Instances of this class must be created
-// destroyed in the controller thread.
+// and destroyed in the controller thread.
 //
-// This class is supplied only for the purpose of testing Google Test's own
+// This class is supplied only for testing Google Test's own
 // constructs. Do not use it in user tests, either directly or indirectly.
 class ThreadStartSemaphore {
  public:
   ThreadStartSemaphore();
   ~ThreadStartSemaphore();
-  // Signals to all test threads created with this semaphore to start. Must
+  // Signals to all threads created with this semaphore to start. Must
-  // be called from the controlling thread.
+  // be called from the controller thread.
   void Signal();
-  // Blocks until the controlling thread signals. Must be called from a test
+  // Blocks until the controller thread signals. Must be called from a test
   // thread.
   void Wait();
 
@@ -947,17 +961,17 @@ class ThreadStartSemaphore {
   GTEST_DISALLOW_COPY_AND_ASSIGN_(ThreadStartSemaphore);
 };
 
-// Helper class for testing Google Test's multithreading constructs.
+// Helper class for testing Google Test's multi-threading constructs.
 // Use:
 //
 //   void ThreadFunc(int param) { /* Do things with param */ }
-// ThreadSemaphore semaphore;
+//   ThreadStartSemaphore semaphore;
 //   ...
 //   // The semaphore parameter is optional; you can supply NULL.
-// ThredWithParam<int> thread(&ThreadFunc, 5, &semaphore);
+//   ThreadWithParam<int> thread(&ThreadFunc, 5, &semaphore);
-// sem.Signal(); // Allows the thread to start.
+//   semaphore.Signal();  // Allows the thread to start.
 //
-// This class is supplied only for the purpose of testing Google Test's own
+// This class is supplied only for testing Google Test's own
 // constructs. Do not use it in user tests, either directly or indirectly.
 template <typename T>
 class ThreadWithParam {
@@ -969,19 +983,16 @@ class ThreadWithParam {
         param_(param),
         start_semaphore_(semaphore),
         finished_(false) {
-    // func_, param_, and start_semaphore_ must be initialized before
+    // The thread can be created only after all fields except thread_
-    // pthread_create() is called.
+    // have been initialized.
-    const int err = pthread_create(&thread_, 0, ThreadMainStatic, this);
+    GTEST_CHECK_POSIX_SUCCESS_(
-    GTEST_CHECK_(err == 0) << "pthread_create failed with error: "
+        pthread_create(&thread_, 0, ThreadMainStatic, this));
-                           << strerror(err) << "(" << err << ")";
   }
   ~ThreadWithParam() { Join(); }
 
   void Join() {
     if (!finished_) {
-      const int err = pthread_join(thread_, 0);
+      GTEST_CHECK_POSIX_SUCCESS_(pthread_join(thread_, 0));
-      GTEST_CHECK_(err == 0) << "pthread_join failed with error:"
-        << strerror(err) << "(" << err << ")";
       finished_ = true;
     }
   }
@@ -992,23 +1003,18 @@ class ThreadWithParam {
       start_semaphore_->Wait();
     func_(param_);
   }
-  static void* ThreadMainStatic(void* param) {
+  static void* ThreadMainStatic(void* thread_with_param) {
-    static_cast<ThreadWithParam<T>*>(param)->ThreadMain();
+    static_cast<ThreadWithParam<T>*>(thread_with_param)->ThreadMain();
-    return NULL;  // We are not interested in thread exit code.
+    return NULL;  // We are not interested in the thread exit code.
   }
 
-  // User supplied thread function.
+  const UserThreadFunc func_;  // User-supplied thread function.
-  const UserThreadFunc func_;
+  const T param_;  // User-supplied parameter to the thread function.
-  // User supplied parameter to UserThreadFunc.
-  const T param_;
-
-  // Native thread object.
-  pthread_t thread_;
   // When non-NULL, used to block execution until the controller thread
   // signals.
   ThreadStartSemaphore* const start_semaphore_;
-  // true iff UserThreadFunc has not completed yet.
+  bool finished_;  // Has the thread function finished?
-  bool finished_;
+  pthread_t thread_;  // The native thread object.
 };
 
 #define GTEST_IS_THREADSAFE 1

src/gtest-port.cc

@@ -81,10 +81,8 @@ const int kStdErrFileno = STDERR_FILENO;
 // newly created test threads until signalled. Instances of this class must
 // be created and destroyed in the controller thread.
 ThreadStartSemaphore::ThreadStartSemaphore() : signalled_(false) {
-  int err = pthread_mutex_init(&mutex_, NULL);
+  GTEST_CHECK_POSIX_SUCCESS_(pthread_mutex_init(&mutex_, NULL));
-  GTEST_CHECK_(err == 0) << "pthread_mutex_init failed with error " << err;
+  GTEST_CHECK_POSIX_SUCCESS_(pthread_cond_init(&cond_, NULL));
-  err = pthread_cond_init(&cond_, NULL);
-  GTEST_CHECK_(err == 0) << "pthread_cond_init failed with error " << err;
   pthread_mutex_lock(&mutex_);
 }
 
@@ -92,75 +90,29 @@ ThreadStartSemaphore::~ThreadStartSemaphore() {
   // Every ThreadStartSemaphore object must be signalled. It locks
   // internal mutex upon creation and Signal unlocks it.
   GTEST_CHECK_(signalled_);
-
+  GTEST_CHECK_POSIX_SUCCESS_(pthread_mutex_destroy(&mutex_));
-  int err = pthread_mutex_destroy(&mutex_);
+  GTEST_CHECK_POSIX_SUCCESS_(pthread_cond_destroy(&cond_));
-  GTEST_CHECK_(err == 0)
-      << "pthread_mutex_destroy failed with error " << err;
-  err = pthread_cond_destroy(&cond_);
-  GTEST_CHECK_(err == 0)
-      << "pthread_cond_destroy failed with error " << err;
 }
 
 // Signals to all test threads to start. Must be called from the
 // controlling thread.
 void ThreadStartSemaphore::Signal() {
   signalled_ = true;
-  int err = pthread_cond_signal(&cond_);
+  GTEST_CHECK_POSIX_SUCCESS_(pthread_cond_signal(&cond_));
-  GTEST_CHECK_(err == 0)
+  GTEST_CHECK_POSIX_SUCCESS_(pthread_mutex_unlock(&mutex_));
-      << "pthread_cond_signal failed with error " << err;
-  err = pthread_mutex_unlock(&mutex_);
-  GTEST_CHECK_(err == 0)
-      << "pthread_mutex_unlock failed with error " << err;
 }
 
 // Blocks until the controlling thread signals. Should be called from a
 // test thread.
 void ThreadStartSemaphore::Wait() {
-  int err = pthread_mutex_lock(&mutex_);
+  GTEST_CHECK_POSIX_SUCCESS_(pthread_mutex_lock(&mutex_));
-  GTEST_CHECK_(err == 0) << "pthread_mutex_lock failed with error " << err;
 
   while (!signalled_) {
-    err = pthread_cond_wait(&cond_, &mutex_);
+    GTEST_CHECK_POSIX_SUCCESS_(pthread_cond_wait(&cond_, &mutex_));
-    GTEST_CHECK_(err == 0)
-        << "pthread_cond_wait failed with error " << err;
-  }
-  err = pthread_mutex_unlock(&mutex_);
-  GTEST_CHECK_(err == 0)
-      << "pthread_mutex_unlock failed with error " << err;
-}
-
-void MutexBase::Lock() {
-  const int err = pthread_mutex_lock(&mutex_);
-  GTEST_CHECK_(err == 0) << "pthread_mutex_lock failed with error " << err;
-  owner_ = pthread_self();
-}
-
-void MutexBase::Unlock() {
-  // We don't protect writing to owner_ here, as it's the caller's
-  // responsibility to ensure that the current thread holds the mutex when
-  // this is called.
-  owner_ = 0;
-  const int err = pthread_mutex_unlock(&mutex_);
-  GTEST_CHECK_(err == 0) << "pthread_mutex_unlock failed with error " << err;
   }
-
+  GTEST_CHECK_POSIX_SUCCESS_(pthread_mutex_unlock(&mutex_));
-// Does nothing if the current thread holds the mutex. Otherwise, crashes
-// with high probability.
-void MutexBase::AssertHeld() const {
-  GTEST_CHECK_(owner_ == pthread_self())
-      << "Current thread is not holding mutex." << this;
 }
 
-Mutex::Mutex() {
-  owner_ = 0;
-  const int err = pthread_mutex_init(&mutex_, NULL);
-  GTEST_CHECK_(err == 0) << "pthread_mutex_init failed with error " << err;
-}
-
-Mutex::~Mutex() {
-  const int err = pthread_mutex_destroy(&mutex_);
-  GTEST_CHECK_(err == 0) << "pthread_mutex_destroy failed with error " << err;
-}
 #endif  // GTEST_HAS_PTHREAD
 
 #if GTEST_OS_MAC

test/gtest-port_test.cc

@@ -770,18 +770,6 @@ TEST(ThreadLocalTest, SingleParamConstructorInitializesToParam) {
   EXPECT_EQ(&i, t2.get());
 }
 
-class NoCopyConstructor {
- public:
-  NoCopyConstructor() {}
- private:
-  GTEST_DISALLOW_COPY_AND_ASSIGN_(NoCopyConstructor);
-};
-
-TEST(ThreadLocalTest, ValueCopyConstructorIsNotRequiredForDefaultVersion) {
-  ThreadLocal<NoCopyConstructor> bar;
-  bar.get();
-}
-
 class NoDefaultContructor {
  public:
   explicit NoDefaultContructor(const char*) {}
@@ -796,9 +784,6 @@ TEST(ThreadLocalTest, ValueDefaultContructorIsNotRequiredForParamVersion) {
 TEST(ThreadLocalTest, GetAndPointerReturnSameValue) {
   ThreadLocal<String> thread_local;
 
-  // This is why EXPECT_TRUE is used here rather than EXPECT_EQ because
-  // we don't care about a particular value of thread_local.pointer() here;
-  // we only care about pointer and reference referring to the same lvalue.
   EXPECT_EQ(thread_local.pointer(), &(thread_local.get()));
 
   // Verifies the condition still holds after calling set.
@@ -825,7 +810,7 @@ TEST(MutexTestDeathTest, AssertHeldShouldAssertWhenNotLocked) {
     { MutexLock lock(&m); }
     m.AssertHeld();
   },
-  "Current thread is not holding mutex..+");
+  "The current thread is not holding the mutex @.+");
 }
 
 void SleepMilliseconds(int time) {
@@ -847,16 +832,13 @@ class AtomicCounterWithMutex {
       // We cannot use Mutex and MutexLock here or rely on their memory
       // barrier functionality as we are testing them here.
       pthread_mutex_t memory_barrier_mutex;
-      int err = pthread_mutex_init(&memory_barrier_mutex, NULL);
+      GTEST_CHECK_POSIX_SUCCESS_(
-      GTEST_CHECK_(err == 0) << "pthread_mutex_init failed with error " << err;
+          pthread_mutex_init(&memory_barrier_mutex, NULL));
-      err = pthread_mutex_lock(&memory_barrier_mutex);
+      GTEST_CHECK_POSIX_SUCCESS_(pthread_mutex_lock(&memory_barrier_mutex));
-      GTEST_CHECK_(err == 0) << "pthread_mutex_lock failed with error " << err;
 
       SleepMilliseconds(random_.Generate(30));
 
-      err = pthread_mutex_unlock(&memory_barrier_mutex);
+      GTEST_CHECK_POSIX_SUCCESS_(pthread_mutex_unlock(&memory_barrier_mutex));
-      GTEST_CHECK_(err == 0)
-          << "pthread_mutex_unlock failed with error " << err;
     }
     value_ = temp + 1;
   }