Repository: thrift Updated Branches: refs/heads/master 7e7a1a7c1 -> 00d425239
THRIFT-3978: tighten up pthread mutex implementation, removing asserts and replacing them with exceptions Client: cpp This closes #1228 Project: http://git-wip-us.apache.org/repos/asf/thrift/repo Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/00d42523 Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/00d42523 Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/00d42523 Branch: refs/heads/master Commit: 00d4252392d9159202cd6ffc4b3294f85265310f Parents: 7e7a1a7 Author: James E. King, III <jk...@apache.org> Authored: Tue Apr 4 09:32:45 2017 -0400 Committer: James E. King, III <jk...@apache.org> Committed: Tue Apr 4 09:32:45 2017 -0400 ---------------------------------------------------------------------- build/cmake/ConfigureChecks.cmake | 1 + lib/cpp/src/thrift/concurrency/BoostMutex.cpp | 4 +- lib/cpp/src/thrift/concurrency/Mutex.cpp | 86 +++++----- lib/cpp/src/thrift/concurrency/Mutex.h | 11 +- lib/cpp/src/thrift/concurrency/StdMutex.cpp | 2 + .../src/thrift/concurrency/ThreadManager.cpp | 4 - lib/cpp/test/CMakeLists.txt | 3 +- lib/cpp/test/Makefile.am | 3 +- lib/cpp/test/RWMutexStarveTest.cpp | 159 ------------------- lib/cpp/test/concurrency/MutexTest.cpp | 123 ++++++++++++++ lib/cpp/test/concurrency/RWMutexStarveTest.cpp | 158 ++++++++++++++++++ 11 files changed, 347 insertions(+), 207 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/thrift/blob/00d42523/build/cmake/ConfigureChecks.cmake ---------------------------------------------------------------------- diff --git a/build/cmake/ConfigureChecks.cmake b/build/cmake/ConfigureChecks.cmake index 81223d8..12a50df 100644 --- a/build/cmake/ConfigureChecks.cmake +++ b/build/cmake/ConfigureChecks.cmake @@ -46,6 +46,7 @@ check_include_file(sys/un.h HAVE_SYS_UN_H) check_include_file(sys/poll.h HAVE_SYS_POLL_H) check_include_file(sys/select.h HAVE_SYS_SELECT_H) check_include_file(sched.h HAVE_SCHED_H) +check_include_file(string.h HAVE_STRING_H) check_include_file(strings.h HAVE_STRINGS_H) check_function_exists(gethostbyname HAVE_GETHOSTBYNAME) http://git-wip-us.apache.org/repos/asf/thrift/blob/00d42523/lib/cpp/src/thrift/concurrency/BoostMutex.cpp ---------------------------------------------------------------------- diff --git a/lib/cpp/src/thrift/concurrency/BoostMutex.cpp b/lib/cpp/src/thrift/concurrency/BoostMutex.cpp index f7cadab..4e556df 100644 --- a/lib/cpp/src/thrift/concurrency/BoostMutex.cpp +++ b/lib/cpp/src/thrift/concurrency/BoostMutex.cpp @@ -33,7 +33,9 @@ namespace thrift { namespace concurrency { /** - * Implementation of Mutex class using boost interprocess mutex + * Implementation of Mutex class using boost::timed_mutex + * + * Methods throw boost::lock_error on error. * * @version $Id:$ */ http://git-wip-us.apache.org/repos/asf/thrift/blob/00d42523/lib/cpp/src/thrift/concurrency/Mutex.cpp ---------------------------------------------------------------------- diff --git a/lib/cpp/src/thrift/concurrency/Mutex.cpp b/lib/cpp/src/thrift/concurrency/Mutex.cpp index b6b915d..bcab05e 100644 --- a/lib/cpp/src/thrift/concurrency/Mutex.cpp +++ b/lib/cpp/src/thrift/concurrency/Mutex.cpp @@ -17,18 +17,24 @@ * under the License. */ +// needed to test for pthread implementation capabilities: +#define __USE_GNU + #include <thrift/thrift-config.h> #include <thrift/Thrift.h> +#include <thrift/concurrency/Exception.h> #include <thrift/concurrency/Mutex.h> #include <thrift/concurrency/Util.h> #include <assert.h> -#ifdef HAVE_PTHREAD_H +#include <stdlib.h> #include <pthread.h> -#endif #include <signal.h> +#include <string.h> +#include <boost/format.hpp> +#include <boost/shared_ptr.hpp> using boost::shared_ptr; namespace apache { @@ -110,9 +116,17 @@ static inline int64_t maybeGetProfilingStartTime() { #define PROFILE_MUTEX_UNLOCKED() #endif // THRIFT_PTHREAD_MUTEX_CONTENTION_PROFILING +#define EINTR_LOOP(_CALL) int ret; do { ret = _CALL; } while (ret == EINTR) +#define ABORT_ONFAIL(_CALL) { EINTR_LOOP(_CALL); if (ret) { abort(); } } +#define THROW_SRE(_CALLSTR, RET) { throw SystemResourceException(boost::str(boost::format("%1% returned %2% (%3%)") % _CALLSTR % RET % ::strerror(RET))); } +#define THROW_SRE_ONFAIL(_CALL) { EINTR_LOOP(_CALL); if (ret) { THROW_SRE(#_CALL, ret); } } +#define THROW_SRE_TRYFAIL(_CALL) { EINTR_LOOP(_CALL); if (ret == 0) { return true; } else if (ret == EBUSY) { return false; } THROW_SRE(#_CALL, ret); } + /** * Implementation of Mutex class using POSIX mutex * + * Throws apache::thrift::concurrency::SystemResourceException on error. + * * @version $Id:$ */ class Mutex::impl { @@ -128,19 +142,19 @@ public: ~impl() { if (initialized_) { initialized_ = false; - int ret = pthread_mutex_destroy(&pthread_mutex_); - THRIFT_UNUSED_VARIABLE(ret); - assert(ret == 0); + ABORT_ONFAIL(pthread_mutex_destroy(&pthread_mutex_)); } } void lock() const { PROFILE_MUTEX_START_LOCK(); - pthread_mutex_lock(&pthread_mutex_); + THROW_SRE_ONFAIL(pthread_mutex_lock(&pthread_mutex_)); PROFILE_MUTEX_LOCKED(); } - bool trylock() const { return (0 == pthread_mutex_trylock(&pthread_mutex_)); } + bool trylock() const { + THROW_SRE_TRYFAIL(pthread_mutex_trylock(&pthread_mutex_)); + } bool timedlock(int64_t milliseconds) const { #if defined(_POSIX_TIMEOUTS) && _POSIX_TIMEOUTS >= 200112L @@ -148,14 +162,16 @@ public: struct THRIFT_TIMESPEC ts; Util::toTimespec(ts, milliseconds + Util::currentTime()); - int ret = pthread_mutex_timedlock(&pthread_mutex_, &ts); + EINTR_LOOP(pthread_mutex_timedlock(&pthread_mutex_, &ts)); if (ret == 0) { PROFILE_MUTEX_LOCKED(); return true; + } else if (ret == ETIMEDOUT) { + PROFILE_MUTEX_NOT_LOCKED(); + return false; } - PROFILE_MUTEX_NOT_LOCKED(); - return false; + THROW_SRE("pthread_mutex_timedlock(&pthread_mutex_, &ts)", ret); #else /* Otherwise follow solution used by Mono for Android */ struct THRIFT_TIMESPEC sleepytime, now, to; @@ -180,7 +196,7 @@ public: void unlock() const { PROFILE_MUTEX_START_UNLOCK(); - pthread_mutex_unlock(&pthread_mutex_); + THROW_SRE_ONFAIL(pthread_mutex_unlock(&pthread_mutex_)); PROFILE_MUTEX_UNLOCKED(); } @@ -219,28 +235,16 @@ void Mutex::unlock() const { void Mutex::DEFAULT_INITIALIZER(void* arg) { pthread_mutex_t* pthread_mutex = (pthread_mutex_t*)arg; - int ret = pthread_mutex_init(pthread_mutex, NULL); - THRIFT_UNUSED_VARIABLE(ret); - assert(ret == 0); + THROW_SRE_ONFAIL(pthread_mutex_init(pthread_mutex, NULL)); } -#if defined(PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP) \ - || defined(PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP) +#if defined(PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP) || defined(PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP) || defined(PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP) static void init_with_kind(pthread_mutex_t* mutex, int kind) { pthread_mutexattr_t mutexattr; - int ret = pthread_mutexattr_init(&mutexattr); - assert(ret == 0); - - // Apparently, this can fail. Should we really be aborting? - ret = pthread_mutexattr_settype(&mutexattr, kind); - assert(ret == 0); - - ret = pthread_mutex_init(mutex, &mutexattr); - assert(ret == 0); - - ret = pthread_mutexattr_destroy(&mutexattr); - assert(ret == 0); - THRIFT_UNUSED_VARIABLE(ret); + THROW_SRE_ONFAIL(pthread_mutexattr_init(&mutexattr)); + THROW_SRE_ONFAIL(pthread_mutexattr_settype(&mutexattr, kind)); + THROW_SRE_ONFAIL(pthread_mutex_init(mutex, &mutexattr)); + THROW_SRE_ONFAIL(pthread_mutexattr_destroy(&mutexattr)); } #endif @@ -258,6 +262,12 @@ void Mutex::ADAPTIVE_INITIALIZER(void* arg) { } #endif +#ifdef PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP +void Mutex::ERRORCHECK_INITIALIZER(void* arg) { + init_with_kind((pthread_mutex_t*)arg, PTHREAD_MUTEX_ERRORCHECK); +} +#endif + #ifdef PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP void Mutex::RECURSIVE_INITIALIZER(void* arg) { init_with_kind((pthread_mutex_t*)arg, PTHREAD_MUTEX_RECURSIVE_NP); @@ -275,40 +285,36 @@ public: #ifdef THRIFT_PTHREAD_MUTEX_CONTENTION_PROFILING profileTime_ = 0; #endif - int ret = pthread_rwlock_init(&rw_lock_, NULL); - THRIFT_UNUSED_VARIABLE(ret); - assert(ret == 0); + THROW_SRE_ONFAIL(pthread_rwlock_init(&rw_lock_, NULL)); initialized_ = true; } ~impl() { if (initialized_) { initialized_ = false; - int ret = pthread_rwlock_destroy(&rw_lock_); - THRIFT_UNUSED_VARIABLE(ret); - assert(ret == 0); + ABORT_ONFAIL(pthread_rwlock_destroy(&rw_lock_)); } } void acquireRead() const { PROFILE_MUTEX_START_LOCK(); - pthread_rwlock_rdlock(&rw_lock_); + THROW_SRE_ONFAIL(pthread_rwlock_rdlock(&rw_lock_)); PROFILE_MUTEX_NOT_LOCKED(); // not exclusive, so use not-locked path } void acquireWrite() const { PROFILE_MUTEX_START_LOCK(); - pthread_rwlock_wrlock(&rw_lock_); + THROW_SRE_ONFAIL(pthread_rwlock_wrlock(&rw_lock_)); PROFILE_MUTEX_LOCKED(); } - bool attemptRead() const { return !pthread_rwlock_tryrdlock(&rw_lock_); } + bool attemptRead() const { THROW_SRE_TRYFAIL(pthread_rwlock_tryrdlock(&rw_lock_)); } - bool attemptWrite() const { return !pthread_rwlock_trywrlock(&rw_lock_); } + bool attemptWrite() const { THROW_SRE_TRYFAIL(pthread_rwlock_trywrlock(&rw_lock_)); } void release() const { PROFILE_MUTEX_START_UNLOCK(); - pthread_rwlock_unlock(&rw_lock_); + THROW_SRE_ONFAIL(pthread_rwlock_unlock(&rw_lock_)); PROFILE_MUTEX_UNLOCKED(); } http://git-wip-us.apache.org/repos/asf/thrift/blob/00d42523/lib/cpp/src/thrift/concurrency/Mutex.h ---------------------------------------------------------------------- diff --git a/lib/cpp/src/thrift/concurrency/Mutex.h b/lib/cpp/src/thrift/concurrency/Mutex.h index 6f892dc..e1e395e 100644 --- a/lib/cpp/src/thrift/concurrency/Mutex.h +++ b/lib/cpp/src/thrift/concurrency/Mutex.h @@ -54,6 +54,11 @@ void enableMutexProfiling(int32_t profilingSampleRate, MutexWaitCallback callbac #endif /** + * NOTE: All mutex implementations throw an exception on failure. See each + * specific implementation to understand the exception type(s) used. + */ + +/** * A simple mutex class * * @version $Id:$ @@ -64,6 +69,7 @@ public: Mutex(Initializer init = DEFAULT_INITIALIZER); virtual ~Mutex() {} + virtual void lock() const; virtual bool trylock() const; virtual bool timedlock(int64_t milliseconds) const; @@ -71,8 +77,11 @@ public: void* getUnderlyingImpl() const; - static void DEFAULT_INITIALIZER(void*); + // If you attempt to use one of these and it fails to link, it means + // your version of pthreads does not support it - try another one. static void ADAPTIVE_INITIALIZER(void*); + static void DEFAULT_INITIALIZER(void*); + static void ERRORCHECK_INITIALIZER(void*); static void RECURSIVE_INITIALIZER(void*); private: http://git-wip-us.apache.org/repos/asf/thrift/blob/00d42523/lib/cpp/src/thrift/concurrency/StdMutex.cpp ---------------------------------------------------------------------- diff --git a/lib/cpp/src/thrift/concurrency/StdMutex.cpp b/lib/cpp/src/thrift/concurrency/StdMutex.cpp index 49c18d8..e0f79fa 100644 --- a/lib/cpp/src/thrift/concurrency/StdMutex.cpp +++ b/lib/cpp/src/thrift/concurrency/StdMutex.cpp @@ -33,6 +33,8 @@ namespace concurrency { /** * Implementation of Mutex class using C++11 std::timed_mutex * + * Methods throw std::system_error on error. + * * @version $Id:$ */ class Mutex::impl : public std::timed_mutex {}; http://git-wip-us.apache.org/repos/asf/thrift/blob/00d42523/lib/cpp/src/thrift/concurrency/ThreadManager.cpp ---------------------------------------------------------------------- diff --git a/lib/cpp/src/thrift/concurrency/ThreadManager.cpp b/lib/cpp/src/thrift/concurrency/ThreadManager.cpp index c4726dd..88cd59a 100644 --- a/lib/cpp/src/thrift/concurrency/ThreadManager.cpp +++ b/lib/cpp/src/thrift/concurrency/ThreadManager.cpp @@ -30,10 +30,6 @@ #include <deque> #include <set> -#if defined(DEBUG) -#include <iostream> -#endif // defined(DEBUG) - namespace apache { namespace thrift { namespace concurrency { http://git-wip-us.apache.org/repos/asf/thrift/blob/00d42523/lib/cpp/test/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/lib/cpp/test/CMakeLists.txt b/lib/cpp/test/CMakeLists.txt index ef3d417..6d4aa5e 100644 --- a/lib/cpp/test/CMakeLists.txt +++ b/lib/cpp/test/CMakeLists.txt @@ -80,7 +80,8 @@ set(UnitTest_SOURCES ) if(NOT WITH_BOOSTTHREADS AND NOT WITH_STDTHREADS AND NOT MSVC AND NOT MINGW) - list(APPEND UnitTest_SOURCES RWMutexStarveTest.cpp) + list(APPEND UnitTest_SOURCES concurrency/MutexTest.cpp) + list(APPEND UnitTest_SOURCES concurrency/RWMutexStarveTest.cpp) endif() add_executable(UnitTests ${UnitTest_SOURCES}) http://git-wip-us.apache.org/repos/asf/thrift/blob/00d42523/lib/cpp/test/Makefile.am ---------------------------------------------------------------------- diff --git a/lib/cpp/test/Makefile.am b/lib/cpp/test/Makefile.am index d387297..f61cff1 100755 --- a/lib/cpp/test/Makefile.am +++ b/lib/cpp/test/Makefile.am @@ -123,7 +123,8 @@ UnitTests_SOURCES = \ if !WITH_BOOSTTHREADS UnitTests_SOURCES += \ - RWMutexStarveTest.cpp + concurrency/MutexTest.cpp \ + concurrency/RWMutexStarveTest.cpp endif UnitTests_LDADD = \ http://git-wip-us.apache.org/repos/asf/thrift/blob/00d42523/lib/cpp/test/RWMutexStarveTest.cpp ---------------------------------------------------------------------- diff --git a/lib/cpp/test/RWMutexStarveTest.cpp b/lib/cpp/test/RWMutexStarveTest.cpp deleted file mode 100644 index 32c1531..0000000 --- a/lib/cpp/test/RWMutexStarveTest.cpp +++ /dev/null @@ -1,159 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -#include <iostream> -#include <unistd.h> - -#include <boost/shared_ptr.hpp> -#include <boost/test/unit_test.hpp> - -#include "thrift/concurrency/Mutex.h" -#include "thrift/concurrency/PosixThreadFactory.h" - -using boost::shared_ptr; -using boost::unit_test::test_suite; -using boost::unit_test::framework::master_test_suite; - -using namespace apache::thrift::concurrency; -using namespace std; - -class Locker : public Runnable { -protected: - Locker(boost::shared_ptr<ReadWriteMutex> rwlock, bool writer) - : rwlock_(rwlock), writer_(writer), started_(false), gotLock_(false), signaled_(false) {} - -public: - virtual void run() { - started_ = true; - if (writer_) { - rwlock_->acquireWrite(); - } else { - rwlock_->acquireRead(); - } - gotLock_ = true; - while (!signaled_) { - usleep(5000); - } - rwlock_->release(); - } - - bool started() const { return started_; } - bool gotLock() const { return gotLock_; } - void signal() { signaled_ = true; } - -protected: - boost::shared_ptr<ReadWriteMutex> rwlock_; - bool writer_; - volatile bool started_; - volatile bool gotLock_; - volatile bool signaled_; -}; - -class Reader : public Locker { -public: - Reader(boost::shared_ptr<ReadWriteMutex> rwlock) : Locker(rwlock, false) {} -}; - -class Writer : public Locker { -public: - Writer(boost::shared_ptr<ReadWriteMutex> rwlock) : Locker(rwlock, true) {} -}; - -void test_starve(PosixThreadFactory::POLICY policy) { - // the man pages for pthread_wrlock_rdlock suggest that any OS guarantee about - // writer starvation may be influenced by the scheduling policy, so let's try - // all 3 policies to see if any of them work. - PosixThreadFactory factory(policy); - factory.setDetached(false); - - boost::shared_ptr<ReadWriteMutex> rwlock(new NoStarveReadWriteMutex()); - - boost::shared_ptr<Reader> reader1(new Reader(rwlock)); - boost::shared_ptr<Reader> reader2(new Reader(rwlock)); - boost::shared_ptr<Writer> writer(new Writer(rwlock)); - - boost::shared_ptr<Thread> treader1 = factory.newThread(reader1); - boost::shared_ptr<Thread> treader2 = factory.newThread(reader2); - boost::shared_ptr<Thread> twriter = factory.newThread(writer); - - // launch a reader and make sure he has the lock - treader1->start(); - while (!reader1->gotLock()) { - usleep(2000); - } - - // launch a writer and make sure he's blocked on the lock - twriter->start(); - while (!writer->started()) { - usleep(2000); - } - // tricky part... we can never be 100% sure that the writer is actually - // blocked on the lock, but we can pretty reasonably sure because we know - // he just executed the line immediately before getting the lock, and - // we'll wait a full second for him to get on it. - sleep(1); - - // launch a second reader... if the RWMutex guarantees that writers won't - // starve, this reader should not be able to acquire the lock until the writer - // has acquired and released it. - treader2->start(); - while (!reader2->started()) { - usleep(2000); - } - // again... can't be 100% sure the reader is waiting on (or has) the lock - // but we can be close. - sleep(1); - - // tell reader 1 to let go of the lock - reader1->signal(); - - // wait for someone to get the lock - while (!reader2->gotLock() && !writer->gotLock()) { - usleep(2000); - } - - // the test succeeded if the WRITER got the lock. - bool success = writer->gotLock(); - - // tell everyone we're done and wait for them to finish - reader2->signal(); - writer->signal(); - treader1->join(); - treader2->join(); - twriter->join(); - - // make sure it worked. - BOOST_CHECK_MESSAGE(success, "writer is starving"); -} - -BOOST_AUTO_TEST_SUITE(RWMutexStarveTest) - -BOOST_AUTO_TEST_CASE(test_starve_other) { - test_starve(PosixThreadFactory::OTHER); -} - -BOOST_AUTO_TEST_CASE(test_starve_rr) { - test_starve(PosixThreadFactory::ROUND_ROBIN); -} - -BOOST_AUTO_TEST_CASE(test_starve_fifo) { - test_starve(PosixThreadFactory::FIFO); -} - -BOOST_AUTO_TEST_SUITE_END() http://git-wip-us.apache.org/repos/asf/thrift/blob/00d42523/lib/cpp/test/concurrency/MutexTest.cpp ---------------------------------------------------------------------- diff --git a/lib/cpp/test/concurrency/MutexTest.cpp b/lib/cpp/test/concurrency/MutexTest.cpp new file mode 100644 index 0000000..781ec1a --- /dev/null +++ b/lib/cpp/test/concurrency/MutexTest.cpp @@ -0,0 +1,123 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// This is linked into the UnitTests test executable + +#include <boost/test/unit_test.hpp> + +#include "thrift/concurrency/Exception.h" +#include "thrift/concurrency/Mutex.h" + +using boost::unit_test::test_suite; +using boost::unit_test::framework::master_test_suite; + +using namespace apache::thrift::concurrency; + +struct LFAT +{ + LFAT() + : uut(Mutex::ERRORCHECK_INITIALIZER) + { + BOOST_CHECK_EQUAL(0, pthread_mutex_init(&mx, 0)); + BOOST_CHECK_EQUAL(0, pthread_cond_init(&cv, 0)); + } + + Mutex uut; + pthread_mutex_t mx; + pthread_cond_t cv; +}; + +// Helper for testing mutex behavior when locked by another thread +void * lockFromAnotherThread(void *ptr) +{ + struct LFAT *lfat = (LFAT *)ptr; + BOOST_CHECK_EQUAL (0, pthread_mutex_lock(&lfat->mx)); // synchronize with testing thread + BOOST_CHECK_NO_THROW( lfat->uut.lock()); + BOOST_CHECK_EQUAL (0, pthread_cond_signal(&lfat->cv)); // tell testing thread we have locked the mutex + BOOST_CHECK_EQUAL (0, pthread_cond_wait(&lfat->cv, &lfat->mx)); // wait for testing thread to signal condition variable telling us to unlock + BOOST_CHECK_NO_THROW( lfat->uut.unlock()); + return ptr; // testing thread should join to ensure completeness +} + +BOOST_AUTO_TEST_SUITE(MutexTest) + +BOOST_AUTO_TEST_CASE(happy_path) +{ + Mutex uut(Mutex::ERRORCHECK_INITIALIZER); // needed to test unlocking twice without undefined behavior + + BOOST_CHECK_NO_THROW( uut.lock()); + BOOST_CHECK_THROW ( uut.lock(), SystemResourceException); // EDEADLK (this thread owns it) + BOOST_CHECK_NO_THROW( uut.unlock()); +} + +BOOST_AUTO_TEST_CASE(recursive_happy_path) +{ + Mutex uut(Mutex::RECURSIVE_INITIALIZER); + + BOOST_CHECK_NO_THROW( uut.lock()); + BOOST_CHECK_NO_THROW( uut.lock()); + BOOST_CHECK_NO_THROW( uut.unlock()); + BOOST_CHECK_NO_THROW( uut.lock()); + BOOST_CHECK_NO_THROW( uut.lock()); + BOOST_CHECK_NO_THROW( uut.unlock()); + BOOST_CHECK_NO_THROW( uut.lock()); + BOOST_CHECK_NO_THROW( uut.unlock()); + BOOST_CHECK_NO_THROW( uut.unlock()); + BOOST_CHECK_NO_THROW( uut.unlock()); +} + +BOOST_AUTO_TEST_CASE(trylock) +{ + Mutex uut(Mutex::ADAPTIVE_INITIALIZER); // just using another initializer for coverage + + BOOST_CHECK ( uut.trylock()); + BOOST_CHECK (!uut.trylock()); + BOOST_CHECK_NO_THROW( uut.unlock()); +} + +BOOST_AUTO_TEST_CASE(timedlock) +{ + pthread_t th; + struct LFAT lfat; + + BOOST_CHECK ( lfat.uut.timedlock(100)); + BOOST_CHECK_THROW ( lfat.uut.timedlock(100), + SystemResourceException); // EDEADLK (current thread owns mutex - logic error) + BOOST_CHECK_NO_THROW( lfat.uut.unlock()); + + BOOST_CHECK_EQUAL (0, pthread_mutex_lock(&lfat.mx)); // synchronize with helper thread + BOOST_CHECK_EQUAL (0, pthread_create(&th, NULL, + lockFromAnotherThread, &lfat)); // create helper thread + BOOST_CHECK_EQUAL (0, pthread_cond_wait(&lfat.cv, &lfat.mx)); // wait for helper thread to lock mutex + + BOOST_CHECK (!lfat.uut.timedlock(100)); // false: another thread owns the lock + + BOOST_CHECK_EQUAL (0, pthread_cond_signal(&lfat.cv)); // tell helper thread we are done + BOOST_CHECK_EQUAL (0, pthread_mutex_unlock(&lfat.mx)); // let helper thread clean up + BOOST_CHECK_EQUAL (0, pthread_join(th, 0)); // wait for testing thread to unlock and be done +} + +BOOST_AUTO_TEST_CASE(underlying) +{ + Mutex uut; + + BOOST_CHECK ( uut.getUnderlyingImpl()); +} + +BOOST_AUTO_TEST_SUITE_END() http://git-wip-us.apache.org/repos/asf/thrift/blob/00d42523/lib/cpp/test/concurrency/RWMutexStarveTest.cpp ---------------------------------------------------------------------- diff --git a/lib/cpp/test/concurrency/RWMutexStarveTest.cpp b/lib/cpp/test/concurrency/RWMutexStarveTest.cpp new file mode 100644 index 0000000..63d780f --- /dev/null +++ b/lib/cpp/test/concurrency/RWMutexStarveTest.cpp @@ -0,0 +1,158 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// This is linked into the UnitTests test executable + +#include <boost/shared_ptr.hpp> +#include <boost/test/unit_test.hpp> + +#include "thrift/concurrency/Mutex.h" +#include "thrift/concurrency/PosixThreadFactory.h" + +using boost::shared_ptr; +using boost::unit_test::test_suite; +using boost::unit_test::framework::master_test_suite; + +using namespace apache::thrift::concurrency; +using namespace std; + +class Locker : public Runnable { +protected: + Locker(boost::shared_ptr<ReadWriteMutex> rwlock, bool writer) + : rwlock_(rwlock), writer_(writer), started_(false), gotLock_(false), signaled_(false) {} + +public: + virtual void run() { + started_ = true; + if (writer_) { + rwlock_->acquireWrite(); + } else { + rwlock_->acquireRead(); + } + gotLock_ = true; + while (!signaled_) { + usleep(5000); + } + rwlock_->release(); + } + + bool started() const { return started_; } + bool gotLock() const { return gotLock_; } + void signal() { signaled_ = true; } + +protected: + boost::shared_ptr<ReadWriteMutex> rwlock_; + bool writer_; + volatile bool started_; + volatile bool gotLock_; + volatile bool signaled_; +}; + +class Reader : public Locker { +public: + Reader(boost::shared_ptr<ReadWriteMutex> rwlock) : Locker(rwlock, false) {} +}; + +class Writer : public Locker { +public: + Writer(boost::shared_ptr<ReadWriteMutex> rwlock) : Locker(rwlock, true) {} +}; + +void test_starve(PosixThreadFactory::POLICY policy) { + // the man pages for pthread_wrlock_rdlock suggest that any OS guarantee about + // writer starvation may be influenced by the scheduling policy, so let's try + // all 3 policies to see if any of them work. + PosixThreadFactory factory(policy); + factory.setDetached(false); + + boost::shared_ptr<ReadWriteMutex> rwlock(new NoStarveReadWriteMutex()); + + boost::shared_ptr<Reader> reader1(new Reader(rwlock)); + boost::shared_ptr<Reader> reader2(new Reader(rwlock)); + boost::shared_ptr<Writer> writer(new Writer(rwlock)); + + boost::shared_ptr<Thread> treader1 = factory.newThread(reader1); + boost::shared_ptr<Thread> treader2 = factory.newThread(reader2); + boost::shared_ptr<Thread> twriter = factory.newThread(writer); + + // launch a reader and make sure he has the lock + treader1->start(); + while (!reader1->gotLock()) { + usleep(2000); + } + + // launch a writer and make sure he's blocked on the lock + twriter->start(); + while (!writer->started()) { + usleep(2000); + } + // tricky part... we can never be 100% sure that the writer is actually + // blocked on the lock, but we can pretty reasonably sure because we know + // he just executed the line immediately before getting the lock, and + // we'll wait a full second for him to get on it. + sleep(1); + + // launch a second reader... if the RWMutex guarantees that writers won't + // starve, this reader should not be able to acquire the lock until the writer + // has acquired and released it. + treader2->start(); + while (!reader2->started()) { + usleep(2000); + } + // again... can't be 100% sure the reader is waiting on (or has) the lock + // but we can be close. + sleep(1); + + // tell reader 1 to let go of the lock + reader1->signal(); + + // wait for someone to get the lock + while (!reader2->gotLock() && !writer->gotLock()) { + usleep(2000); + } + + // the test succeeded if the WRITER got the lock. + bool success = writer->gotLock(); + + // tell everyone we're done and wait for them to finish + reader2->signal(); + writer->signal(); + treader1->join(); + treader2->join(); + twriter->join(); + + // make sure it worked. + BOOST_CHECK_MESSAGE(success, "writer is starving"); +} + +BOOST_AUTO_TEST_SUITE(RWMutexStarveTest) + +BOOST_AUTO_TEST_CASE(test_starve_other) { + test_starve(PosixThreadFactory::OTHER); +} + +BOOST_AUTO_TEST_CASE(test_starve_rr) { + test_starve(PosixThreadFactory::ROUND_ROBIN); +} + +BOOST_AUTO_TEST_CASE(test_starve_fifo) { + test_starve(PosixThreadFactory::FIFO); +} + +BOOST_AUTO_TEST_SUITE_END()
