[2202] Implement the mutex

And fix the tests. Some of them were broken a lot, some of them only
little.

src/lib/util/threads/lock.cc

@@ -13,3 +13,129 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include "lock.h"
+
+#include <exceptions/exceptions.h>
+
+#include <cstring>
+#include <memory>
+#include <cerrno>
+#include <cassert>
+
+#include <pthread.h>
+
+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> 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));
+    }
+}
+
+}
+}
+}

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> mutex(new Mutex);
+    Mutex* mutex = new Mutex;
-    Mutex::Locker locker(*mutex);
+    Mutex::Locker* locker = new Mutex::Locker(*mutex);
-    // Note: This maybe leaks somewhere. But this is a test for development aid
+    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.
+    // it means there's a bug. And we can't delete the locker now, since it
-    EXPECT_THROW(mutex.reset(), isc::InvalidOperation);
+    // 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 length = 1000;
-const unsigned long long iterations = 1000000;
+const unsigned long long iterations = 10000;
-const unsigned long long value = 200000;
+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] ++;

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) {
+doSomething(int* x) {
-    EXPECT_FALSE(*mark);
+    delete[] x;
-    *mark = true;
 }
 
 // We just test that we can forget about the thread and nothing
 // bad will happen on our side.
 TEST(ThreadTest, detached) {
-    bool mark = false;
+    for (size_t i = 0; i < detached_iterations; ++ i) {
-    Thread thread(boost::bind(markRun, &mark));
+        int* x = new int[10];
-    // Can't check the mark - the thread might not have been run yet.
+        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