From: Michal 'vorner' Vaner Date: Tue, 4 Sep 2012 09:15:42 +0000 (+0200) Subject: [2202] Implement the mutex X-Git-Tag: trac2402_base~79^2~22 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=35d60f15a86733bb3bc9db77b3d8c7713d83d70d;p=thirdparty%2Fkea.git [2202] Implement the mutex And fix the tests. Some of them were broken a lot, some of them only little. --- diff --git a/src/lib/util/threads/lock.cc b/src/lib/util/threads/lock.cc index ece77f5939..86c4463aad 100644 --- a/src/lib/util/threads/lock.cc +++ b/src/lib/util/threads/lock.cc @@ -13,3 +13,129 @@ // PERFORMANCE OF THIS SOFTWARE. #include "lock.h" + +#include + +#include +#include +#include +#include + +#include + +using namespace std; + +namespace isc { +namespace util { +namespace thread { + +class Mutex::Impl { +public: + Impl() : + locked(0) + {} + pthread_mutex_t mutex; + // Only in debug mode + size_t locked; +}; + +namespace { + +struct Deinitializer { + Deinitializer(pthread_mutexattr_t& attributes): + attributes_(attributes) + {} + ~ Deinitializer() { + int result = pthread_mutexattr_destroy(&attributes_); + if (result != 0) { + // This really should not happen. We might as well + // try to use assert here. + isc_throw(isc::InvalidOperation, strerror(result)); + } + } + pthread_mutexattr_t& attributes_; +}; + +} + +Mutex::Mutex(bool recursive) : + impl_(NULL) +{ + pthread_mutexattr_t attributes; + int result = pthread_mutexattr_init(&attributes); + switch (result) { + case 0: // All 0K + break; + case ENOMEM: + throw std::bad_alloc(); + default: + isc_throw(isc::InvalidOperation, strerror(result)); + } + Deinitializer deinitializer(attributes); + // TODO: Distinguish if debug mode is enabled in compilation. + // If so, it should be PTHREAD_MUTEX_NORMAL or NULL + int type = PTHREAD_MUTEX_ERRORCHECK; + if (recursive) { + type = PTHREAD_MUTEX_RECURSIVE; + } + // Stop on first error you find and store the result. + result = pthread_mutexattr_settype(&attributes, type) || + pthread_mutexattr_setrobust(&attributes, PTHREAD_MUTEX_ROBUST); + if (result != 0) { + isc_throw(isc::InvalidOperation, strerror(result)); + } + auto_ptr impl(new Impl); + result = pthread_mutex_init(&impl->mutex, &attributes); + switch (result) { + case 0: // All 0K + impl_ = impl.release(); + break; + case ENOMEM: + case EAGAIN: + throw std::bad_alloc(); + default: + isc_throw(isc::InvalidOperation, strerror(result)); + } +} + +Mutex::~ Mutex() { + if (impl_ != NULL) { + int result = pthread_mutex_destroy(&impl_->mutex); + bool locked = impl_->locked != 0; + delete impl_; + if (result != 0) { + // Yes, really throwing from the destructor. + // But the error should not happen during normal + // operations, this means something is screwed up + // and must be fixed. + isc_throw(isc::InvalidOperation, strerror(result)); + } + if (locked) { + isc_throw(isc::InvalidOperation, "Destroying locked mutex"); + } + } +} + +void +Mutex::lock() { + assert(impl_ != NULL); + int result = pthread_mutex_lock(&impl_->mutex); + if (result != 0) { + isc_throw(isc::InvalidOperation, strerror(result)); + } + impl_->locked ++; // Only in debug mode +} + +void +Mutex::unlock() { + assert(impl_ != NULL); + impl_->locked --; // Only in debug mode + int result = pthread_mutex_unlock(&impl_->mutex); + if (result != 0) { + isc_throw(isc::InvalidOperation, strerror(result)); + } +} + +} +} +} diff --git a/src/lib/util/threads/tests/lock_unittest.cc b/src/lib/util/threads/tests/lock_unittest.cc index 2d90ee7301..c2755cf127 100644 --- a/src/lib/util/threads/tests/lock_unittest.cc +++ b/src/lib/util/threads/tests/lock_unittest.cc @@ -49,12 +49,13 @@ TEST(MutexTest, lockMultiple) { TEST(MutexTest, destroyLocked) { // TODO: This probably won't work for non-debug mutexes. Disable on non-debug // compilation. - auto_ptr mutex(new Mutex); - Mutex::Locker locker(*mutex); - // Note: This maybe leaks somewhere. But this is a test for development aid + Mutex* mutex = new Mutex; + Mutex::Locker* locker = new Mutex::Locker(*mutex); + EXPECT_THROW(delete mutex, isc::InvalidOperation); + // Note: This maybe leaks the locker. But this is a test for development aid // exception. The exception won't happen in normal build anyway and seeing - // it means there's a bug. - EXPECT_THROW(mutex.reset(), isc::InvalidOperation); + // it means there's a bug. And we can't delete the locker now, since it + // would access uninitialized memory. } // This test tries if a mutex really locks. We could try that with a deadlock, @@ -73,9 +74,9 @@ TEST(MutexTest, destroyLocked) { // one will add something to the position another one have chosen and // the other one will then zero it, not taking the new value into account. // That'd lower the total value of the array. -const unsigned long long length = 100000; -const unsigned long long iterations = 1000000; -const unsigned long long value = 200000; +const unsigned long long length = 1000; +const unsigned long long iterations = 10000; +const unsigned long long value = 2000; void performStrangeOperation(long long unsigned* array, int direction, Mutex* mutex) @@ -90,7 +91,7 @@ performStrangeOperation(long long unsigned* array, int direction, unsigned long long p2 = position; while (value > 0) { p2 += direction; - if (p2 == position) { + if (p2 % length == position % length) { continue; } array[p2 % length] ++; diff --git a/src/lib/util/threads/tests/thread_unittest.cc b/src/lib/util/threads/tests/thread_unittest.cc index 5e67bf8757..7ba162b49b 100644 --- a/src/lib/util/threads/tests/thread_unittest.cc +++ b/src/lib/util/threads/tests/thread_unittest.cc @@ -25,28 +25,35 @@ // We run some tests mutiple times to see if there happen to be a race // condition (then it would have better chance showing up). // -// The detached tests are not run multiple times to prevent many threads being +// The detached tests are not run as many times to prevent many threads being // started in parallel (the other tests wait for the previous one to terminate // before starting new one). -const size_t iterations = 100; +const size_t iterations = 200; +const size_t detached_iterations = 25; using namespace isc::util::thread; namespace { void -markRun(bool* mark) { - EXPECT_FALSE(*mark); - *mark = true; +doSomething(int* x) { + delete[] x; } // We just test that we can forget about the thread and nothing // bad will happen on our side. TEST(ThreadTest, detached) { - bool mark = false; - Thread thread(boost::bind(markRun, &mark)); - // Can't check the mark - the thread might not have been run yet. + for (size_t i = 0; i < detached_iterations; ++ i) { + int* x = new int[10]; + Thread thread(boost::bind(&doSomething, x)); + } +} + +void +markRun(bool* mark) { + EXPECT_FALSE(*mark); + *mark = true; } // Wait for a thread to end first. The variable must be set at the time. @@ -68,7 +75,9 @@ throwSomething() { // Exception in the thread we forget about should not do anything to us TEST(ThreadTest, detachedException) { - Thread thread(throwSomething); + for (size_t i = 0; i < detached_iterations; ++ i) { + Thread thread(throwSomething); + } } // An uncaught exception in the thread should propagate through wait