Parcourir la source

[master] Merge remote-tracking branch 'origin/trac2669'

Jelte Jansen il y a 12 ans
Parent
commit
5958767f0f

+ 3 - 4
src/lib/asiolink/tests/tcp_socket_unittest.cc

@@ -82,7 +82,7 @@ public:
     struct PrivateData {
         PrivateData() :
             error_code_(), length_(0), cumulative_(0), expected_(0), offset_(0),
-            name_(""), queued_(NONE), called_(NONE)
+            name_(""), queued_(NONE), called_(NONE), data_(MIN_SIZE, 0)
         {}
 
         asio::error_code    error_code_;    ///< Completion error code
@@ -93,8 +93,7 @@ public:
         std::string         name_;          ///< Which of the objects this is
         Operation           queued_;        ///< Queued operation
         Operation           called_;        ///< Which callback called
-        uint8_t             data_[MIN_SIZE];  ///< Receive buffer
-
+        std::vector<uint8_t> data_;  ///< Receive buffer
     };
 
     /// \brief Constructor
@@ -169,7 +168,7 @@ public:
 
     /// \brief Get data member
     uint8_t* data() {
-        return (ptr_->data_);
+        return (&ptr_->data_[0]);
     }
 
     /// \brief Get flag to say what was queued

+ 71 - 68
src/lib/server_common/tests/socket_requestor_test.cc

@@ -35,6 +35,7 @@
 
 #include <util/io/fd.h>
 #include <util/io/fd_share.h>
+#include <util/unittests/check_valgrind.h>
 
 using namespace isc::data;
 using namespace isc::config;
@@ -509,81 +510,83 @@ private:
 };
 
 TEST_F(SocketRequestorTest, testSocketPassing) {
-    TestSocket ts;
-    std::vector<std::pair<std::string, int> > data;
-    data.push_back(std::pair<std::string, int>("foo\n", 1));
-    data.push_back(std::pair<std::string, int>("bar\n", 2));
-    data.push_back(std::pair<std::string, int>("foo\n", 3));
-    data.push_back(std::pair<std::string, int>("foo\n", 1));
-    data.push_back(std::pair<std::string, int>("foo\n", -1));
-    data.push_back(std::pair<std::string, int>("foo\n", -2));
-
-    // run() returns true iff we can specify read timeout so we avoid a
-    // deadlock.  Unless there's a bug the test should succeed even without the
-    // timeout, but we don't want to make the test hang up in case with an
-    // unexpected bug, so we'd rather skip most of the tests in that case.
-    const bool timo_ok = ts.run(data);
-    SocketRequestor::SocketID socket_id;
-    if (timo_ok) {
-        // 1 should be ok
-        addAnswer("foo", ts.getPath());
-        socket_id = doRequest();
-        EXPECT_EQ("foo", socket_id.second);
-        EXPECT_EQ(0, close(socket_id.first));
-
-        // 2 should be ok too
-        addAnswer("bar", ts.getPath());
-        socket_id = doRequest();
-        EXPECT_EQ("bar", socket_id.second);
-        EXPECT_EQ(0, close(socket_id.first));
-
-        // 3 should be ok too (reuse earlier token)
-        addAnswer("foo", ts.getPath());
-        socket_id = doRequest();
-        EXPECT_EQ("foo", socket_id.second);
-        EXPECT_EQ(0, close(socket_id.first));
-    }
-    // Create a second socket server, to test that multiple different
-    // domains sockets would work as well (even though we don't actually
-    // use that feature)
-    TestSocket ts2;
-    std::vector<std::pair<std::string, int> > data2;
-    data2.push_back(std::pair<std::string, int>("foo\n", 1));
-    const bool timo_ok2 = ts2.run(data2);
-
-    if (timo_ok2) {
-        // 1 should be ok
-        addAnswer("foo", ts2.getPath());
-        socket_id = doRequest();
-        EXPECT_EQ("foo", socket_id.second);
-        EXPECT_EQ(0, close(socket_id.first));
-    }
+    if (!isc::util::unittests::runningOnValgrind()) {
+        TestSocket ts;
+        std::vector<std::pair<std::string, int> > data;
+        data.push_back(std::pair<std::string, int>("foo\n", 1));
+        data.push_back(std::pair<std::string, int>("bar\n", 2));
+        data.push_back(std::pair<std::string, int>("foo\n", 3));
+        data.push_back(std::pair<std::string, int>("foo\n", 1));
+        data.push_back(std::pair<std::string, int>("foo\n", -1));
+        data.push_back(std::pair<std::string, int>("foo\n", -2));
+
+        // run() returns true iff we can specify read timeout so we avoid a
+        // deadlock.  Unless there's a bug the test should succeed even without the
+        // timeout, but we don't want to make the test hang up in case with an
+        // unexpected bug, so we'd rather skip most of the tests in that case.
+        const bool timo_ok = ts.run(data);
+        SocketRequestor::SocketID socket_id;
+        if (timo_ok) {
+            // 1 should be ok
+            addAnswer("foo", ts.getPath());
+            socket_id = doRequest();
+            EXPECT_EQ("foo", socket_id.second);
+            EXPECT_EQ(0, close(socket_id.first));
+
+            // 2 should be ok too
+            addAnswer("bar", ts.getPath());
+            socket_id = doRequest();
+            EXPECT_EQ("bar", socket_id.second);
+            EXPECT_EQ(0, close(socket_id.first));
+
+            // 3 should be ok too (reuse earlier token)
+            addAnswer("foo", ts.getPath());
+            socket_id = doRequest();
+            EXPECT_EQ("foo", socket_id.second);
+            EXPECT_EQ(0, close(socket_id.first));
+        }
+        // Create a second socket server, to test that multiple different
+        // domains sockets would work as well (even though we don't actually
+        // use that feature)
+        TestSocket ts2;
+        std::vector<std::pair<std::string, int> > data2;
+        data2.push_back(std::pair<std::string, int>("foo\n", 1));
+        const bool timo_ok2 = ts2.run(data2);
+
+        if (timo_ok2) {
+            // 1 should be ok
+            addAnswer("foo", ts2.getPath());
+            socket_id = doRequest();
+            EXPECT_EQ("foo", socket_id.second);
+            EXPECT_EQ(0, close(socket_id.first));
+        }
 
-    if (timo_ok) {
-        // Now use first socket again
-        addAnswer("foo", ts.getPath());
-        socket_id = doRequest();
-        EXPECT_EQ("foo", socket_id.second);
-        EXPECT_EQ(0, close(socket_id.first));
+        if (timo_ok) {
+            // Now use first socket again
+            addAnswer("foo", ts.getPath());
+            socket_id = doRequest();
+            EXPECT_EQ("foo", socket_id.second);
+            EXPECT_EQ(0, close(socket_id.first));
+
+            // -1 is a "normal" error
+            addAnswer("foo", ts.getPath());
+            EXPECT_THROW(doRequest(), SocketRequestor::SocketError);
+
+            // -2 is an unexpected error.  After this point it's not guaranteed the
+            // connection works as intended.
+            addAnswer("foo", ts.getPath());
+            EXPECT_THROW(doRequest(), SocketRequestor::SocketError);
+        }
 
-        // -1 is a "normal" error
+        // Vector is of first socket is now empty, so the socket should be gone
         addAnswer("foo", ts.getPath());
         EXPECT_THROW(doRequest(), SocketRequestor::SocketError);
 
-        // -2 is an unexpected error.  After this point it's not guaranteed the
-        // connection works as intended.
-        addAnswer("foo", ts.getPath());
+        // Vector is of second socket is now empty too, so the socket should be
+        // gone
+        addAnswer("foo", ts2.getPath());
         EXPECT_THROW(doRequest(), SocketRequestor::SocketError);
     }
-
-    // Vector is of first socket is now empty, so the socket should be gone
-    addAnswer("foo", ts.getPath());
-    EXPECT_THROW(doRequest(), SocketRequestor::SocketError);
-
-    // Vector is of second socket is now empty too, so the socket should be
-    // gone
-    addAnswer("foo", ts2.getPath());
-    EXPECT_THROW(doRequest(), SocketRequestor::SocketError);
 }
 
 }

+ 41 - 37
src/lib/util/tests/fd_share_tests.cc

@@ -15,6 +15,7 @@
 #include <util/io/fd.h>
 #include <util/io/fd_share.h>
 
+#include <util/unittests/check_valgrind.h>
 #include <util/unittests/fork.h>
 
 #include <gtest/gtest.h>
@@ -30,44 +31,47 @@ namespace {
 
 // We test that we can transfer a pipe over other pipe
 TEST(FDShare, transfer) {
-    // Get a pipe and fork
-    int pipes[2];
-    ASSERT_NE(-1, socketpair(AF_UNIX, SOCK_STREAM, 0, pipes));
-    pid_t sender(fork());
-    ASSERT_NE(-1, sender);
-    if(sender) { // We are in parent
-        // Close the other side of pipe, we want only writible one
-        EXPECT_NE(-1, close(pipes[0]));
-        // Get a process to check data
-        int fd(0);
-        pid_t checker(check_output(&fd, "data", 4));
-        ASSERT_NE(-1, checker);
-        // Now, send the file descriptor, close it and close the pipe
-        EXPECT_NE(-1, send_fd(pipes[1], fd));
-        EXPECT_NE(-1, close(pipes[1]));
-        EXPECT_NE(-1, close(fd));
-        // Check both subprocesses ended well
-        EXPECT_TRUE(process_ok(sender));
-        EXPECT_TRUE(process_ok(checker));
-    } else { // We are in child. We do not use ASSERT here
-        // Close the write end, we only read
-        if(close(pipes[1])) {
-            exit(1);
-        }
-        // Get the file descriptor
-        int fd(recv_fd(pipes[0]));
-        if(fd == -1) {
-            exit(1);
-        }
-        // This pipe is not needed
-        if(close(pipes[0])) {
-            exit(1);
-        }
-        // Send "data" trough the received fd, close it and be done
-        if(!write_data(fd, "data", 4) || close(fd) == -1) {
-            exit(1);
+
+    if (!isc::util::unittests::runningOnValgrind()) {
+        // Get a pipe and fork
+        int pipes[2];
+        ASSERT_NE(-1, socketpair(AF_UNIX, SOCK_STREAM, 0, pipes));
+        const pid_t sender(fork());
+        ASSERT_NE(-1, sender);
+        if (sender) { // We are in parent
+            // Close the other side of pipe, we want only writible one
+            EXPECT_NE(-1, close(pipes[0]));
+            // Get a process to check data
+            int fd(0);
+            const pid_t checker(check_output(&fd, "data", 4));
+            ASSERT_NE(-1, checker);
+            // Now, send the file descriptor, close it and close the pipe
+            EXPECT_NE(-1, send_fd(pipes[1], fd));
+            EXPECT_NE(-1, close(pipes[1]));
+            EXPECT_NE(-1, close(fd));
+            // Check both subprocesses ended well
+            EXPECT_TRUE(process_ok(sender));
+            EXPECT_TRUE(process_ok(checker));
+        } else { // We are in child. We do not use ASSERT here
+            // Close the write end, we only read
+            if (close(pipes[1])) {
+                exit(1);
+            }
+            // Get the file descriptor
+            const int fd(recv_fd(pipes[0]));
+            if (fd == -1) {
+                exit(1);
+            }
+            // This pipe is not needed
+            if (close(pipes[0])) {
+                exit(1);
+            }
+            // Send "data" trough the received fd, close it and be done
+            if (!write_data(fd, "data", 4) || close(fd) == -1) {
+                exit(1);
+            }
+            exit(0);
         }
-        exit(0);
     }
 }
 

+ 20 - 14
src/lib/util/tests/fd_tests.cc

@@ -12,6 +12,8 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <util/unittests/check_valgrind.h>
+
 #include <util/io/fd.h>
 
 #include <util/unittests/fork.h>
@@ -45,24 +47,28 @@ class FDTest : public ::testing::Test {
 
 // Test we read what was sent
 TEST_F(FDTest, read) {
-    int read_pipe(0);
-    buffer = new unsigned char[TEST_DATA_SIZE];
-    pid_t feeder(provide_input(&read_pipe, data, TEST_DATA_SIZE));
-    ASSERT_GE(feeder, 0);
-    ssize_t received(read_data(read_pipe, buffer, TEST_DATA_SIZE));
-    EXPECT_TRUE(process_ok(feeder));
-    EXPECT_EQ(TEST_DATA_SIZE, received);
-    EXPECT_EQ(0, memcmp(data, buffer, received));
+    if (!isc::util::unittests::runningOnValgrind()) {
+        int read_pipe(0);
+        buffer = new unsigned char[TEST_DATA_SIZE];
+        pid_t feeder(provide_input(&read_pipe, data, TEST_DATA_SIZE));
+        ASSERT_GE(feeder, 0);
+        ssize_t received(read_data(read_pipe, buffer, TEST_DATA_SIZE));
+        EXPECT_TRUE(process_ok(feeder));
+        EXPECT_EQ(TEST_DATA_SIZE, received);
+        EXPECT_EQ(0, memcmp(data, buffer, received));
+    }
 }
 
 // Test we write the correct thing
 TEST_F(FDTest, write) {
-    int write_pipe(0);
-    pid_t checker(check_output(&write_pipe, data, TEST_DATA_SIZE));
-    ASSERT_GE(checker, 0);
-    EXPECT_TRUE(write_data(write_pipe, data, TEST_DATA_SIZE));
-    EXPECT_EQ(0, close(write_pipe));
-    EXPECT_TRUE(process_ok(checker));
+    if (!isc::util::unittests::runningOnValgrind()) {
+        int write_pipe(0);
+        pid_t checker(check_output(&write_pipe, data, TEST_DATA_SIZE));
+        ASSERT_GE(checker, 0);
+        EXPECT_TRUE(write_data(write_pipe, data, TEST_DATA_SIZE));
+        EXPECT_EQ(0, close(write_pipe));
+        EXPECT_TRUE(process_ok(checker));
+    }
 }
 
 }

+ 77 - 68
src/lib/util/tests/interprocess_sync_file_unittest.cc

@@ -13,6 +13,8 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include "util/interprocess_sync_file.h"
+
+#include <util/unittests/check_valgrind.h>
 #include <gtest/gtest.h>
 #include <unistd.h>
 
@@ -53,59 +55,62 @@ parentReadLockedState (int fd) {
 }
 
 TEST(InterprocessSyncFileTest, TestLock) {
-  InterprocessSyncFile sync("test");
-  InterprocessSyncLocker locker(sync);
+    InterprocessSyncFile sync("test");
+    InterprocessSyncLocker locker(sync);
 
-  EXPECT_FALSE(locker.isLocked());
-  EXPECT_TRUE(locker.lock());
-  EXPECT_TRUE(locker.isLocked());
+    EXPECT_FALSE(locker.isLocked());
+    EXPECT_TRUE(locker.lock());
+    EXPECT_TRUE(locker.isLocked());
 
-  int fds[2];
+    if (!isc::util::unittests::runningOnValgrind()) {
 
-  // Here, we check that a lock has been taken by forking and
-  // checking from the child that a lock exists. This has to be
-  // done from a separate process as we test by trying to lock the
-  // range again on the lock file. The lock attempt would pass if
-  // done from the same process for the granted range. The lock
-  // attempt must fail to pass our check.
+        int fds[2];
 
-  EXPECT_EQ(0, pipe(fds));
+        // Here, we check that a lock has been taken by forking and
+        // checking from the child that a lock exists. This has to be
+        // done from a separate process as we test by trying to lock the
+        // range again on the lock file. The lock attempt would pass if
+        // done from the same process for the granted range. The lock
+        // attempt must fail to pass our check.
 
-  if (fork() == 0) {
-      unsigned char locked = 0;
-      // Child writes to pipe
-      close(fds[0]);
+        EXPECT_EQ(0, pipe(fds));
 
-      InterprocessSyncFile sync2("test");
-      InterprocessSyncLocker locker2(sync2);
+        if (fork() == 0) {
+            unsigned char locked = 0;
+            // Child writes to pipe
+            close(fds[0]);
 
-      if (!locker2.tryLock()) {
-          EXPECT_FALSE(locker2.isLocked());
-          locked = 1;
-      } else {
-          EXPECT_TRUE(locker2.isLocked());
-      }
+            InterprocessSyncFile sync2("test");
+            InterprocessSyncLocker locker2(sync2);
 
-      ssize_t bytes_written = write(fds[1], &locked, sizeof(locked));
-      EXPECT_EQ(sizeof(locked), bytes_written);
+            if (!locker2.tryLock()) {
+                EXPECT_FALSE(locker2.isLocked());
+                locked = 1;
+            } else {
+                EXPECT_TRUE(locker2.isLocked());
+            }
 
-      close(fds[1]);
-      exit(0);
-  } else {
-      // Parent reads from pipe
-      close(fds[1]);
+            ssize_t bytes_written = write(fds[1], &locked, sizeof(locked));
+            EXPECT_EQ(sizeof(locked), bytes_written);
 
-      const unsigned char locked = parentReadLockedState(fds[0]);
+            close(fds[1]);
+            exit(0);
+        } else {
+            // Parent reads from pipe
+            close(fds[1]);
 
-      close(fds[0]);
+            const unsigned char locked = parentReadLockedState(fds[0]);
 
-      EXPECT_EQ(1, locked);
-  }
+            close(fds[0]);
 
-  EXPECT_TRUE(locker.unlock());
-  EXPECT_FALSE(locker.isLocked());
+            EXPECT_EQ(1, locked);
+        }
+    }
+
+    EXPECT_TRUE(locker.unlock());
+    EXPECT_FALSE(locker.isLocked());
 
-  EXPECT_EQ (0, unlink(TEST_DATA_TOPBUILDDIR "/test_lockfile"));
+    EXPECT_EQ (0, unlink(TEST_DATA_TOPBUILDDIR "/test_lockfile"));
 }
 
 TEST(InterprocessSyncFileTest, TestMultipleFilesDirect) {
@@ -126,49 +131,53 @@ TEST(InterprocessSyncFileTest, TestMultipleFilesDirect) {
 }
 
 TEST(InterprocessSyncFileTest, TestMultipleFilesForked) {
-  InterprocessSyncFile sync("test1");
-  InterprocessSyncLocker locker(sync);
+    InterprocessSyncFile sync("test1");
+    InterprocessSyncLocker locker(sync);
 
-  EXPECT_TRUE(locker.lock());
+    EXPECT_TRUE(locker.lock());
 
-  int fds[2];
+    if (!isc::util::unittests::runningOnValgrind()) {
 
-  EXPECT_EQ(0, pipe(fds));
+        int fds[2];
 
-  if (fork() == 0) {
-      unsigned char locked = 0xff;
-      // Child writes to pipe
-      close(fds[0]);
+        EXPECT_EQ(0, pipe(fds));
 
-      InterprocessSyncFile sync2("test2");
-      InterprocessSyncLocker locker2(sync2);
+        if (fork() == 0) {
+            unsigned char locked = 0xff;
+            // Child writes to pipe
+            close(fds[0]);
 
-      if (locker2.tryLock()) {
-          locked = 0;
-      }
+            InterprocessSyncFile sync2("test2");
+            InterprocessSyncLocker locker2(sync2);
 
-      ssize_t bytes_written = write(fds[1], &locked, sizeof(locked));
-      EXPECT_EQ(sizeof(locked), bytes_written);
+            if (locker2.tryLock()) {
+                locked = 0;
+            }
 
-      close(fds[1]);
-      exit(0);
-  } else {
-      // Parent reads from pipe
-      close(fds[1]);
+            ssize_t bytes_written = write(fds[1], &locked, sizeof(locked));
+            EXPECT_EQ(sizeof(locked), bytes_written);
 
-      const unsigned char locked = parentReadLockedState(fds[0]);
+            close(fds[1]);
+            exit(0);
+        } else {
+            // Parent reads from pipe
+            close(fds[1]);
 
-      close(fds[0]);
+            const unsigned char locked = parentReadLockedState(fds[0]);
 
-      EXPECT_EQ(0, locked);
-  }
+            close(fds[0]);
 
-  EXPECT_TRUE(locker.unlock());
+            EXPECT_EQ(0, locked);
+        }
 
-  EXPECT_EQ (0, unlink(TEST_DATA_TOPBUILDDIR "/test1_lockfile"));
-  EXPECT_EQ (0, unlink(TEST_DATA_TOPBUILDDIR "/test2_lockfile"));
-}
+        EXPECT_EQ (0, unlink(TEST_DATA_TOPBUILDDIR "/test2_lockfile"));
+    }
+
+    EXPECT_TRUE(locker.unlock());
+
+    EXPECT_EQ (0, unlink(TEST_DATA_TOPBUILDDIR "/test1_lockfile"));
 }
 
+} // anonymous namespace
 } // namespace util
 } // namespace isc

+ 17 - 12
src/lib/util/threads/tests/condvar_unittest.cc

@@ -15,6 +15,7 @@
 #include <config.h>
 
 #include <exceptions/exceptions.h>
+#include <util/unittests/check_valgrind.h>
 
 #include <util/threads/sync.h>
 #include <util/threads/thread.h>
@@ -84,12 +85,14 @@ ringSignal(CondVar* condvar, Mutex* mutex, int* arg) {
 
 // A simple wait-signal operation on a condition variable.
 TEST_F(CondVarTest, waitAndSignal) {
-    Mutex::Locker locker(mutex_);
-    int shared_var = 0; // let the other thread increment this
-    Thread t(boost::bind(&ringSignal, &condvar_, &mutex_, &shared_var));
-    condvar_.wait(mutex_);
-    t.wait();
-    EXPECT_EQ(1, shared_var);
+    if (!isc::util::unittests::runningOnValgrind()) {
+        Mutex::Locker locker(mutex_);
+        int shared_var = 0; // let the other thread increment this
+        Thread t(boost::bind(&ringSignal, &condvar_, &mutex_, &shared_var));
+        condvar_.wait(mutex_);
+        t.wait();
+        EXPECT_EQ(1, shared_var);
+    }
 }
 
 // Thread's main code for the next test
@@ -143,12 +146,14 @@ signalAndWait(CondVar* condvar, Mutex* mutex) {
 TEST_F(CondVarTest, destroyWhileWait) {
     // We'll destroy a CondVar object while the thread is still waiting
     // on it.  This will trigger an assertion failure.
-    EXPECT_DEATH_IF_SUPPORTED({
-            CondVar cond;
-            Mutex::Locker locker(mutex_);
-            Thread t(boost::bind(&signalAndWait, &cond, &mutex_));
-            cond.wait(mutex_);
-        }, "");
+    if (!isc::util::unittests::runningOnValgrind()) {
+        EXPECT_DEATH_IF_SUPPORTED({
+                CondVar cond;
+                Mutex::Locker locker(mutex_);
+                Thread t(boost::bind(&signalAndWait, &cond, &mutex_));
+                cond.wait(mutex_);
+            }, "");
+    }
 }
 #endif // !HAS_UNDEFINED_PTHREAD_BEHAVIOR
 

+ 29 - 27
src/lib/util/threads/tests/lock_unittest.cc

@@ -94,33 +94,35 @@ void
 noHandler(int) {}
 
 TEST(MutexTest, swarm) {
-    // Create a timeout in case something got stuck here
-    struct sigaction ignored, original;
-    memset(&ignored, 0, sizeof(ignored));
-    ignored.sa_handler = noHandler;
-    if (sigaction(SIGALRM, &ignored, &original)) {
-        FAIL() << "Couldn't set alarm";
-    }
-    alarm(10);
-    // This type has a low chance of being atomic itself, further raising
-    // the chance of problems appearing.
-    double canary = 0;
-    Mutex mutex;
-    // Run two parallel threads
-    bool ready1 = false;
-    bool ready2 = false;
-    Thread t1(boost::bind(&performIncrement, &canary, &ready1, &ready2,
-                          &mutex));
-    Thread t2(boost::bind(&performIncrement, &canary, &ready2, &ready1,
-                          &mutex));
-    t1.wait();
-    t2.wait();
-    // Check it the sum is the expected value.
-    EXPECT_EQ(iterations * 2, canary) << "Threads are badly synchronized";
-    // Cancel the alarm and return the original handler
-    alarm(0);
-    if (sigaction(SIGALRM, &original, NULL)) {
-        FAIL() << "Couldn't restore alarm";
+    if (!isc::util::unittests::runningOnValgrind()) {
+        // Create a timeout in case something got stuck here
+        struct sigaction ignored, original;
+        memset(&ignored, 0, sizeof(ignored));
+        ignored.sa_handler = noHandler;
+        if (sigaction(SIGALRM, &ignored, &original)) {
+            FAIL() << "Couldn't set alarm";
+        }
+        alarm(10);
+        // This type has a low chance of being atomic itself, further raising
+        // the chance of problems appearing.
+        double canary = 0;
+        Mutex mutex;
+        // Run two parallel threads
+        bool ready1 = false;
+        bool ready2 = false;
+        Thread t1(boost::bind(&performIncrement, &canary, &ready1, &ready2,
+                              &mutex));
+        Thread t2(boost::bind(&performIncrement, &canary, &ready2, &ready1,
+                              &mutex));
+        t1.wait();
+        t2.wait();
+        // Check it the sum is the expected value.
+        EXPECT_EQ(iterations * 2, canary) << "Threads are badly synchronized";
+        // Cancel the alarm and return the original handler
+        alarm(0);
+        if (sigaction(SIGALRM, &original, NULL)) {
+            FAIL() << "Couldn't restore alarm";
+        }
     }
 }
 

+ 29 - 20
src/lib/util/threads/tests/thread_unittest.cc

@@ -13,6 +13,7 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <util/threads/thread.h>
+#include <util/unittests/check_valgrind.h>
 
 #include <boost/bind.hpp>
 
@@ -41,9 +42,11 @@ doSomething(int*) { }
 // We just test that we can forget about the thread and nothing
 // bad will happen on our side.
 TEST(ThreadTest, detached) {
-    int x;
-    for (size_t i = 0; i < detached_iterations; ++i) {
-        Thread thread(boost::bind(&doSomething, &x));
+    if (!isc::util::unittests::runningOnValgrind()) {
+        int x;
+        for (size_t i = 0; i < detached_iterations; ++i) {
+            Thread thread(boost::bind(&doSomething, &x));
+        }
     }
 }
 
@@ -55,13 +58,15 @@ markRun(bool* mark) {
 
 // Wait for a thread to end first. The variable must be set at the time.
 TEST(ThreadTest, wait) {
-    for (size_t i = 0; i < iterations; ++i) {
-        bool mark = false;
-        Thread thread(boost::bind(markRun, &mark));
-        thread.wait();
-        ASSERT_TRUE(mark) << "Not finished yet in " << i << "th iteration";
-        // Can't wait second time
-        ASSERT_THROW(thread.wait(), isc::InvalidOperation);
+    if (!isc::util::unittests::runningOnValgrind()) {
+        for (size_t i = 0; i < iterations; ++i) {
+            bool mark = false;
+            Thread thread(boost::bind(markRun, &mark));
+            thread.wait();
+            ASSERT_TRUE(mark) << "Not finished yet in " << i << "th iteration";
+            // Can't wait second time
+            ASSERT_THROW(thread.wait(), isc::InvalidOperation);
+        }
     }
 }
 
@@ -77,21 +82,25 @@ throwException() {
 
 // Exception in the thread we forget about should not do anything to us
 TEST(ThreadTest, detachedException) {
-    for (size_t i = 0; i < detached_iterations; ++i) {
-        Thread thread(throwSomething);
-    }
-    for (size_t i = 0; i < detached_iterations; ++i) {
-        Thread thread(throwException);
+    if (!isc::util::unittests::runningOnValgrind()) {
+        for (size_t i = 0; i < detached_iterations; ++i) {
+            Thread thread(throwSomething);
+        }
+        for (size_t i = 0; i < detached_iterations; ++i) {
+            Thread thread(throwException);
+        }
     }
 }
 
 // An uncaught exception in the thread should propagate through wait
 TEST(ThreadTest, exception) {
-    for (size_t i = 0; i < iterations; ++i) {
-        Thread thread(throwSomething);
-        Thread thread2(throwException);
-        ASSERT_THROW(thread.wait(), Thread::UncaughtException);
-        ASSERT_THROW(thread2.wait(), Thread::UncaughtException);
+    if (!isc::util::unittests::runningOnValgrind()) {
+        for (size_t i = 0; i < iterations; ++i) {
+            Thread thread(throwSomething);
+            Thread thread2(throwException);
+            ASSERT_THROW(thread.wait(), Thread::UncaughtException);
+            ASSERT_THROW(thread2.wait(), Thread::UncaughtException);
+        }
     }
 }
 

+ 5 - 0
src/lib/util/unittests/check_valgrind.h

@@ -38,6 +38,11 @@ namespace unittests {
 
 /// \brief Check if the program is run in valgrind
 ///
+/// This is used to check for valgrind and skip (parts of) tests that fork,
+/// such as death tests, and general forking tests, and some threading tests;
+/// These tend to cause valgrind to report errors, which would hide other
+/// potential valgrind reports.
+///
 /// \return true if valgrind headers are available, and valgrind is running,
 ///         false if the headers are not available, or if valgrind is not
 ///         running

+ 2 - 0
src/lib/util/unittests/fork.cc

@@ -77,6 +77,7 @@ provide_input(int *read_pipe, const void *input, const size_t length)
         return -1;
     }
     *read_pipe = pipes[0];
+
     pid_t pid(fork());
     if (pid) { // We are in the parent
         return pid;
@@ -91,6 +92,7 @@ provide_input(int *read_pipe, const void *input, const size_t length)
     }
 }
 
+
 /*
  * This creates a pipe, forks and reads the pipe and compares it
  * with given data. Used to check output of run in an asynchronous way.

+ 12 - 11
tests/tools/perfdhcp/test_control.cc

@@ -13,27 +13,28 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 
-#include <fstream>
-#include <stdio.h>
-#include <stdlib.h>
-#include <stdint.h>
-#include <unistd.h>
-#include <signal.h>
-#include <sys/wait.h>
-
-#include <boost/date_time/posix_time/posix_time.hpp>
-
 #include <exceptions/exceptions.h>
 #include <asiolink/io_address.h>
 #include <dhcp/libdhcp++.h>
 #include <dhcp/iface_mgr.h>
 #include <dhcp/dhcp4.h>
 #include <dhcp/option6_ia.h>
+#include <util/unittests/check_valgrind.h>
 #include "test_control.h"
 #include "command_options.h"
 #include "perf_pkt4.h"
 #include "perf_pkt6.h"
 
+#include <fstream>
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <unistd.h>
+#include <signal.h>
+#include <sys/wait.h>
+
+#include <boost/date_time/posix_time/posix_time.hpp>
+
 using namespace std;
 using namespace boost::posix_time;
 using namespace isc;
@@ -724,7 +725,7 @@ TestControl::printDiagnostics() const {
         std::cout << "Set MAC to " << vector2Hex(options.getMacTemplate(), "::")
                   << std::endl;
         if (options.getDuidTemplate().size() > 0) {
-            std::cout << "Set DUID to " << vector2Hex(options.getDuidTemplate()) << std::endl; 
+            std::cout << "Set DUID to " << vector2Hex(options.getDuidTemplate()) << std::endl;
         }
     }
 }