Repository: thrift Updated Branches: refs/heads/master a718ad483 -> 237a394ad
THRIFT-3974: fix ThreadSanitizer identified issues Client: C++ This closes #1331 Project: http://git-wip-us.apache.org/repos/asf/thrift/repo Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/237a394a Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/237a394a Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/237a394a Branch: refs/heads/master Commit: 237a394add04ce02cc274836c0ec1c7260fdaadd Parents: a718ad4 Author: James E. King, III <jk...@apache.org> Authored: Sat Aug 12 13:04:55 2017 -0700 Committer: James E. King, III <jk...@apache.org> Committed: Sat Aug 12 16:15:59 2017 -0700 ---------------------------------------------------------------------- .../thrift/concurrency/PosixThreadFactory.cpp | 34 ++++++++++++++------ lib/cpp/src/thrift/transport/TFileTransport.h | 2 +- lib/cpp/test/TServerIntegrationTest.cpp | 2 +- 3 files changed, 27 insertions(+), 11 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/thrift/blob/237a394a/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 @@ public: private: pthread_t pthread_; + Mutex state_mutex_; STATE state_; int policy_; int priority_; @@ -70,7 +72,6 @@ public: #ifndef _WIN32 pthread_(0), #endif // _WIN32 - state_(uninitialized), policy_(policy), priority_(priority), @@ -93,8 +94,20 @@ public: } } + 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 @@ public: 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 @@ public: } 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 @@ void* PthreadThread::threadMain(void* arg) { return (void*)0; } - if (thread->state_ != starting) { + if (thread->getState() != starting) { return (void*)0; } @@ -201,10 +214,13 @@ void* PthreadThread::threadMain(void* arg) { 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; http://git-wip-us.apache.org/repos/asf/thrift/blob/237a394a/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 @@ private: // 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_; http://git-wip-us.apache.org/repos/asf/thrift/blob/237a394a/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 @@ public: 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; };
