BIND 10 trac1704_2, updated. c0a328b2d94321966d5ebe97dd8f2058d6d76176 [1704] Unit test the logger for the lock

BIND 10 source code commits bind10-changes at lists.isc.org
Thu May 24 07:27:28 UTC 2012


The branch, trac1704_2 has been updated
       via  c0a328b2d94321966d5ebe97dd8f2058d6d76176 (commit)
       via  b45a54e8ccf0c947b7f3979ed1083260c47be453 (commit)
       via  8c455b3a39a588e11a86523056caa9c379b60a67 (commit)
       via  31b153df2003831950bf3fcd6a4da782dc579eb1 (commit)
       via  10b434be10f3fa900760180feb030907484c9b6f (commit)
       via  db2fa0cd177d28c49a9c99aabe44bd9c47c6b78d (commit)
       via  09e8c9e8c55571196fbd297fda46e95400aa0990 (commit)
       via  2bf7dad904d16760987d3a0b27c02bb8a2b50e55 (commit)
      from  876dd74d15bcaa619602bcb60fc769d541923048 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit c0a328b2d94321966d5ebe97dd8f2058d6d76176
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu May 24 12:48:43 2012 +0530

    [1704] Unit test the logger for the lock

commit b45a54e8ccf0c947b7f3979ed1083260c47be453
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu May 24 12:16:29 2012 +0530

    [1704] Open lock files lazily

commit 8c455b3a39a588e11a86523056caa9c379b60a67
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu May 24 11:49:18 2012 +0530

    [1704] Add a select() to the parent reader in case the child messes up

commit 31b153df2003831950bf3fcd6a4da782dc579eb1
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu May 24 11:36:45 2012 +0530

    [1704] Replace TEST_F with TEST

commit 10b434be10f3fa900760180feb030907484c9b6f
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu May 24 11:32:22 2012 +0530

    [1704] Minimize code

commit db2fa0cd177d28c49a9c99aabe44bd9c47c6b78d
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu May 24 11:19:59 2012 +0530

    [1704] fd_ cannot be -1 here

commit 09e8c9e8c55571196fbd297fda46e95400aa0990
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu May 24 11:18:34 2012 +0530

    [1704] Use a helper for lock/unlock/tryLock as they use the same pattern

commit 2bf7dad904d16760987d3a0b27c02bb8a2b50e55
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu May 24 11:03:45 2012 +0530

    [1704] Remove separate class hierarchy for InterprocessSyncLocker

-----------------------------------------------------------------------

Summary of changes:
 src/lib/log/logger.cc                              |    7 +
 src/lib/log/logger.h                               |    7 +
 src/lib/log/logger_impl.cc                         |   15 +-
 src/lib/log/logger_impl.h                          |    8 +-
 src/lib/log/tests/Makefile.am                      |    3 +
 src/lib/log/tests/log_test_messages.cc             |   25 ++++
 src/lib/log/tests/log_test_messages.h              |   16 ++
 .../lib/log/tests/log_test_messages.mes            |   24 ++-
 src/lib/log/tests/logger_unittest.cc               |   58 ++++++++
 src/lib/util/interprocess_sync.h                   |   39 +++--
 src/lib/util/interprocess_sync_file.cc             |  154 +++++++-------------
 src/lib/util/interprocess_sync_file.h              |   27 +---
 .../util/tests/interprocess_sync_file_unittest.cc  |  122 ++++++++--------
 13 files changed, 291 insertions(+), 214 deletions(-)
 create mode 100644 src/lib/log/tests/log_test_messages.cc
 create mode 100644 src/lib/log/tests/log_test_messages.h
 copy tests/system/ixfr/clean_ns.sh => src/lib/log/tests/log_test_messages.mes (55%)

-----------------------------------------------------------------------
diff --git a/src/lib/log/logger.cc b/src/lib/log/logger.cc
index d10e979..7f434d4 100644
--- a/src/lib/log/logger.cc
+++ b/src/lib/log/logger.cc
@@ -73,6 +73,13 @@ Logger::getEffectiveSeverity() {
     return (getLoggerPtr()->getEffectiveSeverity());
 }
 
+// Replace the interprocess synchronization object
+
+void
+Logger::setInterprocessSync(isc::util::InterprocessSync* sync) {
+    getLoggerPtr()->setInterprocessSync(sync);
+}
+
 // Debug level (only relevant if messages of severity DEBUG are being logged).
 
 int
diff --git a/src/lib/log/logger.h b/src/lib/log/logger.h
index 5715bc4..e579f87 100644
--- a/src/lib/log/logger.h
+++ b/src/lib/log/logger.h
@@ -25,6 +25,7 @@
 #include <log/message_types.h>
 #include <log/log_formatter.h>
 
+#include <util/interprocess_sync.h>
 
 namespace isc {
 namespace log {
@@ -178,6 +179,12 @@ public:
     /// is the severity of the parent.
     virtual isc::log::Severity getEffectiveSeverity();
 
+    /// \brief Replace the interprocess synchronization object
+    ///
+    /// \param sync The logger uses this synchronization object for
+    /// synchronizing output of log messages.
+    void setInterprocessSync(isc::util::InterprocessSync* sync);
+
     /// \brief Return DEBUG Level
     ///
     /// \return Current setting of debug level.  This is returned regardless of
diff --git a/src/lib/log/logger_impl.cc b/src/lib/log/logger_impl.cc
index c735033..dff8b3e 100644
--- a/src/lib/log/logger_impl.cc
+++ b/src/lib/log/logger_impl.cc
@@ -33,6 +33,7 @@
 #include <log/message_types.h>
 
 #include <util/strutil.h>
+#include <util/interprocess_sync_file.h>
 
 // Note: as log4cplus and the BIND 10 logger have many concepts in common, and
 // thus many similar names, to disambiguate types we don't "use" the log4cplus
@@ -75,6 +76,14 @@ LoggerImpl::getSeverity() {
     return level.severity;
 }
 
+// Replace the interprocess synchronization object
+
+void
+LoggerImpl::setInterprocessSync(isc::util::InterprocessSync* sync) {
+    delete sync_;
+    sync_ = sync;
+}
+
 // Return current debug level (only valid if current severity level is DEBUG).
 int
 LoggerImpl::getDebugLevel() {
@@ -111,9 +120,9 @@ LoggerImpl::outputRaw(const Severity& severity, const string& message) {
     // Use a lock file for mutual exclusion from other processes to
     // avoid log messages getting interspersed
 
-    auto_ptr<InterprocessSyncLocker> locker(sync_->getLocker());
+    InterprocessSyncLocker locker(*sync_);
 
-    if (!locker->lock()) {
+    if (!locker.lock()) {
         LOG4CPLUS_ERROR(logger_, "Unable to lock logger lockfile");
     }
 
@@ -138,7 +147,7 @@ LoggerImpl::outputRaw(const Severity& severity, const string& message) {
             LOG4CPLUS_FATAL(logger_, message);
     }
 
-    if (!locker->unlock()) {
+    if (!locker.unlock()) {
         LOG4CPLUS_ERROR(logger_, "Unable to unlock logger lockfile");
     }
 }
diff --git a/src/lib/log/logger_impl.h b/src/lib/log/logger_impl.h
index bad15ba..94a68f7 100644
--- a/src/lib/log/logger_impl.h
+++ b/src/lib/log/logger_impl.h
@@ -32,7 +32,7 @@
 #include <log/logger_level_impl.h>
 #include <log/message_types.h>
 
-#include <util/interprocess_sync_file.h>
+#include <util/interprocess_sync.h>
 
 namespace isc {
 namespace log {
@@ -110,6 +110,12 @@ public:
     virtual Severity getEffectiveSeverity();
 
 
+    /// \brief Replace the interprocess synchronization object
+    ///
+    /// \param sync The logger uses this synchronization object for
+    /// synchronizing output of log messages.
+    void setInterprocessSync(isc::util::InterprocessSync* sync);
+
     /// \brief Return debug level
     ///
     /// \return Current setting of debug level.  This will be zero if the
diff --git a/src/lib/log/tests/Makefile.am b/src/lib/log/tests/Makefile.am
index dd3c08c..8d53aad 100644
--- a/src/lib/log/tests/Makefile.am
+++ b/src/lib/log/tests/Makefile.am
@@ -13,6 +13,8 @@ endif
 
 CLEANFILES = *.gcno *.gcda
 
+EXTRA_DIST = log_test_messages.mes
+
 noinst_PROGRAMS = logger_example
 logger_example_SOURCES = logger_example.cc
 logger_example_CPPFLAGS = $(AM_CPPFLAGS)
@@ -59,6 +61,7 @@ run_unittests_SOURCES += logger_manager_unittest.cc
 run_unittests_SOURCES += logger_name_unittest.cc
 run_unittests_SOURCES += logger_support_unittest.cc
 run_unittests_SOURCES += logger_unittest.cc
+run_unittests_SOURCES += log_test_messages.cc log_test_messages.h
 run_unittests_SOURCES += logger_specification_unittest.cc
 run_unittests_SOURCES += message_dictionary_unittest.cc
 run_unittests_SOURCES += message_reader_unittest.cc
diff --git a/src/lib/log/tests/log_test_messages.cc b/src/lib/log/tests/log_test_messages.cc
new file mode 100644
index 0000000..e6ee3a0
--- /dev/null
+++ b/src/lib/log/tests/log_test_messages.cc
@@ -0,0 +1,25 @@
+// File created from log_test_messages.mes on Thu May 24 12:52:20 2012
+
+#include <cstddef>
+#include <log/message_types.h>
+#include <log/message_initializer.h>
+
+namespace isc {
+namespace log {
+
+extern const isc::log::MessageID LOG_LOCK_TEST_MESSAGE = "LOG_LOCK_TEST_MESSAGE";
+
+} // namespace log
+} // namespace isc
+
+namespace {
+
+const char* values[] = {
+    "LOG_LOCK_TEST_MESSAGE", "this is a test message.",
+    NULL
+};
+
+const isc::log::MessageInitializer initializer(values);
+
+} // Anonymous namespace
+
diff --git a/src/lib/log/tests/log_test_messages.h b/src/lib/log/tests/log_test_messages.h
new file mode 100644
index 0000000..30023cf
--- /dev/null
+++ b/src/lib/log/tests/log_test_messages.h
@@ -0,0 +1,16 @@
+// File created from log_test_messages.mes on Thu May 24 12:52:20 2012
+
+#ifndef __LOG_TEST_MESSAGES_H
+#define __LOG_TEST_MESSAGES_H
+
+#include <log/message_types.h>
+
+namespace isc {
+namespace log {
+
+extern const isc::log::MessageID LOG_LOCK_TEST_MESSAGE;
+
+} // namespace log
+} // namespace isc
+
+#endif // __LOG_TEST_MESSAGES_H
diff --git a/src/lib/log/tests/log_test_messages.mes b/src/lib/log/tests/log_test_messages.mes
new file mode 100644
index 0000000..ed4940c
--- /dev/null
+++ b/src/lib/log/tests/log_test_messages.mes
@@ -0,0 +1,26 @@
+# Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+#
+# Permission to use, copy, modify, and/or distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+# REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+# AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+# LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+# OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+# PERFORMANCE OF THIS SOFTWARE.
+
+# \brief Message Utility Message File
+#
+# This is the source of the set of messages generated by the message and
+# logging components.  The associated .h and .cc files are created by hand from
+# this file though and are not built during the build process; this is to avoid
+# the chicken-and-egg situation where we need the files to build the message
+# compiler, yet we need the compiler to build the files.
+
+$NAMESPACE isc::log
+
+% LOG_LOCK_TEST_MESSAGE this is a test message.
+This is a log message used in testing.
diff --git a/src/lib/log/tests/logger_unittest.cc b/src/lib/log/tests/logger_unittest.cc
index 069205e..3b980c8 100644
--- a/src/lib/log/tests/logger_unittest.cc
+++ b/src/lib/log/tests/logger_unittest.cc
@@ -23,6 +23,9 @@
 #include <log/logger_manager.h>
 #include <log/logger_name.h>
 #include <log/log_messages.h>
+#include "log/tests/log_test_messages.h"
+
+#include <util/interprocess_sync_file.h>
 
 using namespace isc;
 using namespace isc::log;
@@ -379,3 +382,58 @@ TEST_F(LoggerTest, LoggerNameLength) {
     }, ".*");
 #endif
 }
+
+class MockSync : public isc::util::InterprocessSync {
+public:
+    /// \brief Constructor
+    MockSync(const std::string component_name) :
+        InterprocessSync(component_name), was_locked_(false), was_unlocked_(false)
+    {}
+
+    bool wasLocked() const {
+        return was_locked_;
+    }
+
+    bool wasUnlocked() const {
+        return was_unlocked_;
+    }
+
+protected:
+    bool lock() {
+        was_locked_ = true;
+        return true;
+    }
+
+    bool tryLock() {
+        return true;
+    }
+
+    bool unlock() {
+        was_unlocked_ = true;
+        return true;
+    }
+
+private:
+    bool was_locked_;
+    bool was_unlocked_;
+};
+
+// Checks that the logger logs exclusively and other BIND 10 components
+// are locked out.
+
+TEST_F(LoggerTest, Lock) {
+    // Create a logger
+    Logger logger("alpha");
+
+    // Setup our own mock sync object so that we can intercept the lock
+    // call and check if a lock has been taken.
+    MockSync *sync = new MockSync("logger");
+    logger.setInterprocessSync(sync);
+
+    // Log a message and put things into play.
+    logger.setSeverity(isc::log::INFO, 100);
+    logger.info(LOG_LOCK_TEST_MESSAGE);
+
+    EXPECT_TRUE(sync->wasLocked());
+    EXPECT_TRUE(sync->wasUnlocked());
+}
diff --git a/src/lib/util/interprocess_sync.h b/src/lib/util/interprocess_sync.h
index cc08d6a..4ef36d5 100644
--- a/src/lib/util/interprocess_sync.h
+++ b/src/lib/util/interprocess_sync.h
@@ -23,39 +23,52 @@ namespace util {
 class InterprocessSyncLocker;
 
 class InterprocessSync {
+friend class InterprocessSyncLocker;
 public:
     /// \brief Constructor
     ///
     /// Creates a interprocess synchronization object
     InterprocessSync(const std::string component_name) :
-        component_name_(component_name)
+        component_name_(component_name), is_locked_(false)
     {}
 
     /// \brief Destructor
     virtual ~InterprocessSync() {}
 
-    virtual InterprocessSyncLocker* getLocker() = 0;
-
 protected:
+    virtual bool lock() = 0;
+    virtual bool tryLock() = 0;
+    virtual bool unlock() = 0;
+
     std::string component_name_;
+    bool is_locked_;
 };
 
 class InterprocessSyncLocker {
-friend class InterprocessSync;
 public:
-    virtual bool lock() = 0;
-    virtual bool tryLock() = 0;
-    virtual bool unlock() = 0;
+    InterprocessSyncLocker(InterprocessSync& sync) :
+        sync_(sync)
+    {}
 
     /// \brief Destructor
-    virtual ~InterprocessSyncLocker() {}
+    ~InterprocessSyncLocker() {
+        unlock();
+    }
+
+    bool lock() {
+        return sync_.lock();
+    }
+
+    bool tryLock() {
+        return sync_.tryLock();
+    }
+
+    bool unlock() {
+        return sync_.unlock();
+    }
 
 protected:
-    InterprocessSyncLocker(InterprocessSync* sync) :
-        sync_(sync), is_locked_(false)
-    {}
-    InterprocessSync* sync_;
-    bool is_locked_;
+    InterprocessSync& sync_;
 };
 
 } // namespace util
diff --git a/src/lib/util/interprocess_sync_file.cc b/src/lib/util/interprocess_sync_file.cc
index 190b84b..73dcfa9 100644
--- a/src/lib/util/interprocess_sync_file.cc
+++ b/src/lib/util/interprocess_sync_file.cc
@@ -26,143 +26,97 @@
 namespace isc {
 namespace util {
 
-InterprocessSyncFile::InterprocessSyncFile(const std::string component_name) :
-    InterprocessSync(component_name)
-{
-    std::string lockfile_path = LOCKFILE_DIR;
-
-    const char* const env = getenv("B10_FROM_SOURCE");
-    if (env != NULL) {
-        lockfile_path = env;
-    }
-
-    const char* const env2 = getenv("B10_FROM_SOURCE_LOCALSTATEDIR");
-    if (env2 != NULL) {
-        lockfile_path = env2;
+InterprocessSyncFile::~InterprocessSyncFile() {
+    if (fd_ != -1) {
+        // This will also release any applied locks.
+        close(fd_);
+        // The lockfile will continue to exist, and we must not delete
+        // it.
     }
+}
 
-    lockfile_path += "/" + component_name_ + "_lockfile";
+bool
+InterprocessSyncFile::do_lock(int cmd, short l_type)
+{
+    // Open lock file only when necessary (i.e., here). This is so that
+    // if a default InterprocessSync object is replaced with another
+    // implementation, it doesn't attempt any opens.
+    if (fd_ == -1) {
+        std::string lockfile_path = LOCKFILE_DIR;
 
-    // Open the lockfile in the constructor so it doesn't do the access
-    // checks every time a message is logged.
-    const mode_t mode = umask(0111);
-    fd_ = open(lockfile_path.c_str(), O_CREAT | O_RDWR, 0660);
-    umask(mode);
+        const char* const env = getenv("B10_FROM_SOURCE");
+        if (env != NULL) {
+            lockfile_path = env;
+        }
 
-    if (fd_ == -1) {
-        isc_throw(InterprocessSyncFileError,
-                  "Unable to use interprocess sync lockfile: " +
-                  lockfile_path);
-    }
-}
+        const char* const env2 = getenv("B10_FROM_SOURCE_LOCALSTATEDIR");
+        if (env2 != NULL) {
+            lockfile_path = env2;
+        }
 
-InterprocessSyncFile::~InterprocessSyncFile() {
-    if (fd_ != -1) {
-        close(fd_);
-    }
+        lockfile_path += "/" + component_name_ + "_lockfile";
 
-    // The lockfile will continue to exist, and we must not delete it.
-}
+        // Open the lockfile in the constructor so it doesn't do the access
+        // checks every time a message is logged.
+        const mode_t mode = umask(0111);
+        fd_ = open(lockfile_path.c_str(), O_CREAT | O_RDWR, 0660);
+        umask(mode);
 
-InterprocessSyncLocker*
-InterprocessSyncFile::getLocker() {
-    InterprocessSyncLocker* locker = new InterprocessSyncFileLocker(this);
-    return (locker);
-}
+        if (fd_ == -1) {
+            isc_throw(InterprocessSyncFileError,
+                      "Unable to use interprocess sync lockfile: " +
+                      lockfile_path);
+        }
+    }
 
-///////////////////////////////////////////////////////////////////////////////////
+    struct flock lock;
 
-InterprocessSyncFileLocker::InterprocessSyncFileLocker(InterprocessSync* sync)
-    : InterprocessSyncLocker(sync)
-{
-}
+    memset(&lock, 0, sizeof (lock));
+    lock.l_type = l_type;
+    lock.l_whence = SEEK_SET;
+    lock.l_start = 0;
+    lock.l_len = 1;
 
-InterprocessSyncFileLocker::~InterprocessSyncFileLocker() {
-    unlock();
+    return (fcntl(fd_, cmd, &lock) == 0);
 }
 
 bool
-InterprocessSyncFileLocker::lock() {
+InterprocessSyncFile::lock() {
     if (is_locked_) {
         return (true);
     }
 
-    InterprocessSyncFile* sync = dynamic_cast<InterprocessSyncFile*>(sync_);
-    const int fd = sync->getFd();
-
-    if (fd != -1) {
-        struct flock lock;
-
-        // Acquire the exclusive lock
-        memset(&lock, 0, sizeof lock);
-        lock.l_type = F_WRLCK;
-        lock.l_whence = SEEK_SET;
-        lock.l_start = 0;
-        lock.l_len = 1;
-
-        const int status = fcntl(fd, F_SETLKW, &lock);
-        if (status == 0) {
-            is_locked_ = true;
-            return (true);
-        }
+    if (do_lock(F_SETLKW, F_WRLCK)) {
+        is_locked_ = true;
+        return (true);
     }
 
     return (false);
 }
 
 bool
-InterprocessSyncFileLocker::tryLock() {
+InterprocessSyncFile::tryLock() {
     if (is_locked_) {
         return (true);
     }
 
-    InterprocessSyncFile* sync = dynamic_cast<InterprocessSyncFile*>(sync_);
-    const int fd = sync->getFd();
-
-    if (fd != -1) {
-        struct flock lock;
-
-        // Acquire the exclusive lock
-        memset(&lock, 0, sizeof lock);
-        lock.l_type = F_WRLCK;
-        lock.l_whence = SEEK_SET;
-        lock.l_start = 0;
-        lock.l_len = 1;
-
-        const int status = fcntl(fd, F_SETLK, &lock);
-        if (status == 0) {
-            is_locked_ = true;
-            return (true);
-        }
+    if (do_lock(F_SETLK, F_WRLCK)) {
+        is_locked_ = true;
+        return (true);
     }
 
     return (false);
 }
 
 bool
-InterprocessSyncFileLocker::unlock() {
+InterprocessSyncFile::unlock() {
     if (!is_locked_) {
         return (true);
     }
 
-    InterprocessSyncFile *sync = dynamic_cast<InterprocessSyncFile*>(sync_);
-    const int fd = sync->getFd();
-
-    if (fd != -1) {
-        struct flock lock;
-
-        // Release the exclusive lock
-        memset(&lock, 0, sizeof lock);
-        lock.l_type = F_UNLCK;
-        lock.l_whence = SEEK_SET;
-        lock.l_start = 0;
-        lock.l_len = 1;
-
-        const int status = fcntl(fd, F_SETLKW, &lock);
-        if (status == 0) {
-            is_locked_ = false;
-            return (true);
-        }
+    if (do_lock(F_SETLKW, F_UNLCK)) {
+        is_locked_ = false;
+        return (true);
     }
 
     return (false);
diff --git a/src/lib/util/interprocess_sync_file.h b/src/lib/util/interprocess_sync_file.h
index a71e4f9..848b736 100644
--- a/src/lib/util/interprocess_sync_file.h
+++ b/src/lib/util/interprocess_sync_file.h
@@ -35,33 +35,22 @@ public:
 class InterprocessSyncFile : public InterprocessSync {
 public:
     /// \brief Constructor
-    InterprocessSyncFile(const std::string component_name);
+    InterprocessSyncFile(const std::string component_name) :
+        InterprocessSync(component_name), fd_(-1)
+    {}
 
     /// \brief Destructor
     ~InterprocessSyncFile();
 
-    InterprocessSyncLocker* getLocker();
-
-    int getFd() const {
-        return (fd_);
-    }
-
-private:
-    int fd_;
-};
-
-class InterprocessSyncFileLocker : public InterprocessSyncLocker {
-friend class InterprocessSyncFile;
-public:
-    /// \brief Destructor
-    ~InterprocessSyncFileLocker();
-
+protected:
     bool lock();
     bool tryLock();
     bool unlock();
 
-protected:
-    InterprocessSyncFileLocker(InterprocessSync* sync);
+private:
+    bool do_lock(int cmd, short l_type);
+
+    int fd_;
 };
 
 } // namespace util
diff --git a/src/lib/util/tests/interprocess_sync_file_unittest.cc b/src/lib/util/tests/interprocess_sync_file_unittest.cc
index 6d9fc32..4030bf7 100644
--- a/src/lib/util/tests/interprocess_sync_file_unittest.cc
+++ b/src/lib/util/tests/interprocess_sync_file_unittest.cc
@@ -20,16 +20,11 @@ using namespace std;
 namespace isc {
 namespace util {
 
-class InterprocessSyncFileTest : public ::testing::Test {
-protected:
-    InterprocessSyncFileTest() {}
-};
+TEST(InterprocessSyncFileTest, TestLock) {
+  InterprocessSyncFile sync("test");
+  InterprocessSyncLocker locker(sync);
 
-TEST_F(InterprocessSyncFileTest, TestLock) {
-  InterprocessSync* sync = new InterprocessSyncFile("test");
-  InterprocessSyncLocker* locker = sync->getLocker();
-
-  EXPECT_TRUE(locker->lock());
+  EXPECT_TRUE(locker.lock());
 
   int fds[2];
 
@@ -40,8 +35,6 @@ TEST_F(InterprocessSyncFileTest, TestLock) {
   // done from the same process for the granted range. The lock
   // attempt must fail to pass our check.
 
-  bool was_locked(false);
-
   pipe(fds);
 
   if (fork() == 0) {
@@ -49,16 +42,13 @@ TEST_F(InterprocessSyncFileTest, TestLock) {
       // Child writes to pipe
       close(fds[0]);
 
-      InterprocessSync* sync2 = new InterprocessSyncFile("test");
-      InterprocessSyncLocker* locker2 = sync2->getLocker();
+      InterprocessSyncFile sync2("test");
+      InterprocessSyncLocker locker2(sync2);
 
-      if (!locker2->tryLock()) {
+      if (!locker2.tryLock()) {
           locked = 1;
       }
 
-      delete locker2;
-      delete sync2;
-
       write(fds[1], &locked, sizeof(locked));
       close(fds[1]);
       exit(0);
@@ -67,53 +57,52 @@ TEST_F(InterprocessSyncFileTest, TestLock) {
       // Parent reads from pipe
       close(fds[1]);
 
-      // Read status and set flag
-      read(fds[0], &locked, sizeof(locked));
-      if (locked == 1) {
-        was_locked = true;
-      } else {
-        was_locked = false;
+      fd_set rfds;
+      FD_ZERO(&rfds);
+      FD_SET(fds[0], &rfds);
+
+      struct timeval tv;
+      tv.tv_sec = 5;
+      tv.tv_usec = 0;
+
+      int nfds = select(fds[0] + 1, &rfds, NULL, NULL, &tv);
+      EXPECT_EQ(1, nfds);
+
+      if (nfds == 1) {
+          // Read status
+          read(fds[0], &locked, sizeof(locked));
       }
 
       close(fds[0]);
-  }
 
-  EXPECT_TRUE(was_locked);
-  EXPECT_TRUE(locker->unlock());
+      EXPECT_EQ(1, locked);
+  }
 
-  delete locker;
-  delete sync;
+  EXPECT_TRUE(locker.unlock());
 }
 
-TEST_F(InterprocessSyncFileTest, TestMultipleFilesDirect) {
-  InterprocessSync* sync = new InterprocessSyncFile("test1");
-  InterprocessSyncLocker* locker = sync->getLocker();
+TEST(InterprocessSyncFileTest, TestMultipleFilesDirect) {
+  InterprocessSyncFile sync("test1");
+  InterprocessSyncLocker locker(sync);
 
-  EXPECT_TRUE(locker->lock());
+  EXPECT_TRUE(locker.lock());
 
-  InterprocessSync* sync2 = new InterprocessSyncFile("test2");
-  InterprocessSyncLocker* locker2 = sync2->getLocker();
-  EXPECT_TRUE(locker2->lock());
-  EXPECT_TRUE(locker2->unlock());
-  delete sync2;
-  delete locker2;
+  InterprocessSyncFile sync2("test2");
+  InterprocessSyncLocker locker2(sync2);
+  EXPECT_TRUE(locker2.lock());
+  EXPECT_TRUE(locker2.unlock());
 
-  EXPECT_TRUE(locker->unlock());
-
-  delete locker;
-  delete sync;
+  EXPECT_TRUE(locker.unlock());
 }
 
-TEST_F(InterprocessSyncFileTest, TestMultipleFilesForked) {
-  InterprocessSync* sync = new InterprocessSyncFile("test");
-  InterprocessSyncLocker* locker = sync->getLocker();
+TEST(InterprocessSyncFileTest, TestMultipleFilesForked) {
+  InterprocessSyncFile sync("test");
+  InterprocessSyncLocker locker(sync);
 
-  EXPECT_TRUE(locker->lock());
+  EXPECT_TRUE(locker.lock());
 
   int fds[2];
 
-  bool was_not_locked(true);
-
   pipe(fds);
 
   if (fork() == 0) {
@@ -121,16 +110,13 @@ TEST_F(InterprocessSyncFileTest, TestMultipleFilesForked) {
       // Child writes to pipe
       close(fds[0]);
 
-      InterprocessSync* sync2 = new InterprocessSyncFile("test2");
-      InterprocessSyncLocker* locker2 = sync2->getLocker();
+      InterprocessSyncFile sync2("test2");
+      InterprocessSyncLocker locker2(sync2);
 
-      if (locker2->tryLock()) {
+      if (locker2.tryLock()) {
           locked = 0;
       }
 
-      delete locker2;
-      delete sync2;
-
       write(fds[1], &locked, sizeof(locked));
       close(fds[1]);
       exit(0);
@@ -139,22 +125,28 @@ TEST_F(InterprocessSyncFileTest, TestMultipleFilesForked) {
       // Parent reads from pipe
       close(fds[1]);
 
-      // Read status and set flag
-      read(fds[0], &locked, sizeof(locked));
-      if (locked == 0) {
-        was_not_locked = true;
-      } else {
-        was_not_locked = false;
+      fd_set rfds;
+      FD_ZERO(&rfds);
+      FD_SET(fds[0], &rfds);
+
+      struct timeval tv;
+      tv.tv_sec = 5;
+      tv.tv_usec = 0;
+
+      int nfds = select(fds[0] + 1, &rfds, NULL, NULL, &tv);
+      EXPECT_EQ(1, nfds);
+
+      if (nfds == 1) {
+          // Read status
+          read(fds[0], &locked, sizeof(locked));
       }
 
       close(fds[0]);
-  }
 
-  EXPECT_TRUE(was_not_locked);
-  EXPECT_TRUE(locker->unlock());
+      EXPECT_EQ(0, locked);
+  }
 
-  delete locker;
-  delete sync;
+  EXPECT_TRUE(locker.unlock());
 }
 
 } // namespace util



More information about the bind10-changes mailing list