commit 237a394add04ce02cc274836c0ec1c7260fdaadd
author James E. King, III <jking@apache.org> Sat Aug 12 13:04:55 2017 -0700
committer James E. King, III <jking@apache.org> Sat Aug 12 16:15:59 2017 -0700
tree 5cdbd7a2be3b76904152b4b20316a2f1d384a227
parent a718ad48378ebcf16fd64adebac7c6988136abbd

THRIFT-3974: fix ThreadSanitizer identified issues
Client: C++

This closes #1331

lib/cpp/src/thrift/concurrency/PosixThreadFactory.cpp

diff --git a/lib/cpp/src/thrift/concurrency/PosixThreadFactory.cpp b/lib/cpp/src/thrift/concurrency/PosixThreadFactory.cpp
index 6bf043b..d829d69 100644
--- a/lib/cpp/src/thrift/concurrency/PosixThreadFactory.cpp
+++ b/lib/cpp/src/thrift/concurrency/PosixThreadFactory.cpp
@@ -19,8 +19,9 @@
 
 #include <thrift/thrift-config.h>
 
-#include <thrift/concurrency/PosixThreadFactory.h>
 #include <thrift/concurrency/Exception.h>
+#include <thrift/concurrency/Mutex.h>
+#include <thrift/concurrency/PosixThreadFactory.h>
 
 #if GOOGLE_PERFTOOLS_REGISTER_THREAD
 #include <google/profiler.h>
@@ -52,6 +53,7 @@
 
 private:
   pthread_t pthread_;
+  Mutex state_mutex_;
   STATE state_;
   int policy_;
   int priority_;
@@ -70,7 +72,6 @@
 #ifndef _WIN32
       pthread_(0),
 #endif // _WIN32
-
       state_(uninitialized),
       policy_(policy),
       priority_(priority),
@@ -93,8 +94,20 @@
     }
   }
 
+  STATE getState() const
+  {
+    Guard g(state_mutex_);
+    return state_;
+  }
+
+  void setState(STATE newState)
+  {
+    Guard g(state_mutex_);
+    state_ = newState;
+  }
+
   void start() {
-    if (state_ != uninitialized) {
+    if (getState() != uninitialized) {
       return;
     }
 
@@ -139,7 +152,7 @@
     stdcxx::shared_ptr<PthreadThread>* selfRef = new stdcxx::shared_ptr<PthreadThread>();
     *selfRef = self_.lock();
 
-    state_ = starting;
+    setState(starting);
 
     if (pthread_create(&pthread_, &thread_attr, threadMain, (void*)selfRef) != 0) {
       throw SystemResourceException("pthread_create failed");
@@ -147,7 +160,7 @@
   }
 
   void join() {
-    if (!detached_ && state_ != uninitialized) {
+    if (!detached_ && getState() != uninitialized) {
       void* ignore;
       /* XXX
          If join fails it is most likely due to the fact
@@ -193,7 +206,7 @@
     return (void*)0;
   }
 
-  if (thread->state_ != starting) {
+  if (thread->getState() != starting) {
     return (void*)0;
   }
 
@@ -201,10 +214,13 @@
   ProfilerRegisterThread();
 #endif
 
-  thread->state_ = started;
+  thread->setState(started);
+
   thread->runnable()->run();
-  if (thread->state_ != stopping && thread->state_ != stopped) {
-    thread->state_ = stopping;
+
+  STATE _s = thread->getState();
+  if (_s != stopping && _s != stopped) {
+    thread->setState(stopping);
   }
 
   return (void*)0;

lib/cpp/src/thrift/transport/TFileTransport.h

diff --git a/lib/cpp/src/thrift/transport/TFileTransport.h b/lib/cpp/src/thrift/transport/TFileTransport.h
index 1167497..d6da436 100644
--- a/lib/cpp/src/thrift/transport/TFileTransport.h
+++ b/lib/cpp/src/thrift/transport/TFileTransport.h
@@ -348,7 +348,7 @@
 
   // conditions used to block when the buffer is full or empty
   Monitor notFull_, notEmpty_;
-  bool closing_;
+  boost::atomic<bool> closing_;
 
   // To keep track of whether the buffer has been flushed
   Monitor flushed_;

lib/cpp/test/TServerIntegrationTest.cpp

diff --git a/lib/cpp/test/TServerIntegrationTest.cpp b/lib/cpp/test/TServerIntegrationTest.cpp
index a6e02f1..12657d4 100644
--- a/lib/cpp/test/TServerIntegrationTest.cpp
+++ b/lib/cpp/test/TServerIntegrationTest.cpp
@@ -320,7 +320,7 @@
   shared_ptr<TServerType> pServer;
   shared_ptr<TServerReadyEventHandler> pEventHandler;
   shared_ptr<boost::thread> pServerThread;
-  bool bStressDone;
+  boost::atomic<bool> bStressDone;
   boost::atomic_int64_t bStressConnectionCount;
   boost::atomic_int64_t bStressRequestCount;
 };