BIND 10 trac1704_2, updated. 7987b09972865cf558dbff4489a198162ec8e3c3 [1704] Use the is_locked_ flag to call unlock() just once
BIND 10 source code commits
bind10-changes at lists.isc.org
Fri Jun 1 07:18:48 UTC 2012
The branch, trac1704_2 has been updated
via 7987b09972865cf558dbff4489a198162ec8e3c3 (commit)
via 1c0e6f80265bb516f5f965eea9b0edac781cbdea (commit)
via 52edf764154ed08161a4683d6b7b7be5a232face (commit)
via cc84b0a02de65a71d80fac9b6f15533e9b0c6037 (commit)
via f68857bea3699eb23c72ddac04a20aa8f42bd551 (commit)
via 07851523e66fe1e045e953d505b97e2cf7f499fc (commit)
via 6306feefed1a1fc462bfff708a47c5fffb351a51 (commit)
via 8d55fd24318cd72d0a624304dfb7c107813e43cf (commit)
via a0bf4ce38c190728f172ee9e2a335248a0ed7a1d (commit)
from 088760558f2321ea60b393b37fba0a672ce9f943 (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 7987b09972865cf558dbff4489a198162ec8e3c3
Author: Mukund Sivaraman <muks at isc.org>
Date: Fri Jun 1 12:17:59 2012 +0530
[1704] Use the is_locked_ flag to call unlock() just once
commit 1c0e6f80265bb516f5f965eea9b0edac781cbdea
Author: Mukund Sivaraman <muks at isc.org>
Date: Fri Jun 1 11:56:57 2012 +0530
[1704] Fix comment describing the test in logger_lock_test.cc
commit 52edf764154ed08161a4683d6b7b7be5a232face
Author: Mukund Sivaraman <muks at isc.org>
Date: Fri Jun 1 11:52:31 2012 +0530
[1704] Remove empty strings used when concatenating to TEST_DATA_TOPBUILDDIR
commit cc84b0a02de65a71d80fac9b6f15533e9b0c6037
Author: Mukund Sivaraman <muks at isc.org>
Date: Fri Jun 1 11:51:12 2012 +0530
[1704] Change search style for includes in the interprocess_sync_* headers
commit f68857bea3699eb23c72ddac04a20aa8f42bd551
Author: Mukund Sivaraman <muks at isc.org>
Date: Fri Jun 1 11:49:29 2012 +0530
[1704] Don't update the environment inside check_environment_unchanged()
This is no longer required.
commit 07851523e66fe1e045e953d505b97e2cf7f499fc
Author: Mukund Sivaraman <muks at isc.org>
Date: Fri Jun 1 11:46:18 2012 +0530
[1704] Clarify why the null logger is used in logger_example
commit 6306feefed1a1fc462bfff708a47c5fffb351a51
Author: Mukund Sivaraman <muks at isc.org>
Date: Fri Jun 1 11:43:40 2012 +0530
[1704] Clarify why a null sync object is used in LoggerManager::readLocalMessageFile()
commit 8d55fd24318cd72d0a624304dfb7c107813e43cf
Author: Mukund Sivaraman <muks at isc.org>
Date: Fri Jun 1 11:40:00 2012 +0530
[1704] Clarify why we update the environment in check_environment_unchanged()
commit a0bf4ce38c190728f172ee9e2a335248a0ed7a1d
Author: Mukund Sivaraman <muks at isc.org>
Date: Fri Jun 1 11:35:38 2012 +0530
[1704] Use CLEANFILES instead of clean-local
-----------------------------------------------------------------------
Summary of changes:
Makefile.am | 3 +--
src/bin/bind10/tests/bind10_test.py.in | 8 +------
src/lib/log/logger_manager.cc | 5 ++++-
src/lib/log/tests/logger_example.cc | 6 ++++-
src/lib/log/tests/logger_lock_test.cc | 11 +++++-----
src/lib/log/tests/logger_lock_test.sh.in | 1 -
src/lib/util/interprocess_sync.h | 18 +++++++++++++--
src/lib/util/interprocess_sync_file.h | 4 ++--
src/lib/util/interprocess_sync_null.cc | 3 +++
src/lib/util/interprocess_sync_null.h | 2 +-
.../util/tests/interprocess_sync_file_unittest.cc | 16 +++++++++-----
.../util/tests/interprocess_sync_null_unittest.cc | 23 ++++++++++++++++----
12 files changed, 68 insertions(+), 32 deletions(-)
-----------------------------------------------------------------------
diff --git a/Makefile.am b/Makefile.am
index c3ed142..ccc5524 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -406,5 +406,4 @@ EXTRA_DIST += ext/coroutine/coroutine.h
pkgconfigdir = $(libdir)/pkgconfig
pkgconfig_DATA = dns++.pc
-clean-local:
- -rm -f $(abs_top_builddir)/logger_lockfile
+CLEANFILES = $(abs_top_builddir)/logger_lockfile
diff --git a/src/bin/bind10/tests/bind10_test.py.in b/src/bin/bind10/tests/bind10_test.py.in
index 5e5afba..3b80fb5 100644
--- a/src/bin/bind10/tests/bind10_test.py.in
+++ b/src/bin/bind10/tests/bind10_test.py.in
@@ -684,13 +684,7 @@ class TestStartStopProcessesBob(unittest.TestCase):
"""
def check_environment_unchanged(self):
# Check whether the environment has not been changed
- cur_os_environ = copy.deepcopy(os.environ)
- if "B10_LOCKFILE_DIR_FROM_BUILD" in original_os_environ:
- original_os_environ.pop("B10_LOCKFILE_DIR_FROM_BUILD")
- if "B10_LOCKFILE_DIR_FROM_BUILD" in cur_os_environ:
- cur_os_environ.pop("B10_LOCKFILE_DIR_FROM_BUILD")
-
- self.assertEqual(original_os_environ, cur_os_environ)
+ self.assertEqual(original_os_environ, os.environ)
def check_started(self, bob, core, auth, resolver):
"""
diff --git a/src/lib/log/logger_manager.cc b/src/lib/log/logger_manager.cc
index ad3bffa..8431c2e 100644
--- a/src/lib/log/logger_manager.cc
+++ b/src/lib/log/logger_manager.cc
@@ -150,7 +150,10 @@ LoggerManager::readLocalMessageFile(const char* file) {
MessageDictionary& dictionary = MessageDictionary::globalDictionary();
MessageReader reader(&dictionary);
- // Turn off use of any lock files
+ // Turn off use of any lock files. This is because this logger can
+ // be used by standalone programs which may not have write access to
+ // the local state directory (to create lock files). So we switch to
+ // using a null interprocess sync object here.
logger.setInterprocessSync(new isc::util::InterprocessSyncNull("logger"));
try {
diff --git a/src/lib/log/tests/logger_example.cc b/src/lib/log/tests/logger_example.cc
index f7274fb..853d48a 100644
--- a/src/lib/log/tests/logger_example.cc
+++ b/src/lib/log/tests/logger_example.cc
@@ -281,7 +281,11 @@ int main(int argc, char** argv) {
LoggerManager::readLocalMessageFile(argv[optind]);
}
- // Log a few messages to different loggers.
+ // Log a few messages to different loggers. Here, we switch to using
+ // null interprocess sync objects for the loggers below as the
+ // logger example can be used as a standalone program (which may not
+ // have write access to a local state directory to create
+ // lockfiles).
isc::log::Logger logger_ex(ROOT_NAME);
logger_ex.setInterprocessSync(new isc::util::InterprocessSyncNull("logger"));
isc::log::Logger logger_alpha("alpha");
diff --git a/src/lib/log/tests/logger_lock_test.cc b/src/lib/log/tests/logger_lock_test.cc
index 4621baf..d63989c 100644
--- a/src/lib/log/tests/logger_lock_test.cc
+++ b/src/lib/log/tests/logger_lock_test.cc
@@ -46,13 +46,12 @@ protected:
}
};
-/// \brief Test InitLogger
+/// \brief Test logger lock sequence
///
-/// A program used in testing the logger that initializes logging using
-/// initLogger(), then outputs several messages at different severities and
-/// debug levels. An external script sets the environment variables and checks
-/// that they have the desired effect.
-
+/// A program used in testing the logger. It verifies that (1) an
+/// interprocess sync lock is first acquired by the logger, (2) the
+/// message is logged by the logger, and (3) the lock is released in
+/// that sequence.
int
main(int, char**) {
initLogger();
diff --git a/src/lib/log/tests/logger_lock_test.sh.in b/src/lib/log/tests/logger_lock_test.sh.in
index a1f5865..0324499 100755
--- a/src/lib/log/tests/logger_lock_test.sh.in
+++ b/src/lib/log/tests/logger_lock_test.sh.in
@@ -34,7 +34,6 @@ cat > $tempfile << .
LOGGER_LOCK_TEST: LOCK
INFO [bind10.log] LOG_LOCK_TEST_MESSAGE this is a test message.
LOGGER_LOCK_TEST: UNLOCK
-LOGGER_LOCK_TEST: UNLOCK
.
rm -f $destfile
B10_LOGGER_SEVERITY=INFO B10_LOGGER_DESTINATION=stdout ./logger_lock_test > $destfile
diff --git a/src/lib/util/interprocess_sync.h b/src/lib/util/interprocess_sync.h
index b04c37f..bcf46c0 100644
--- a/src/lib/util/interprocess_sync.h
+++ b/src/lib/util/interprocess_sync.h
@@ -38,6 +38,10 @@ class InterprocessSyncLocker; // forward declaration
/// 4. Client performs task that needs mutual exclusion.
/// 5. Client frees lock with unlock(), or simply returns from the basic
/// block which forms the scope for the InterprocessSyncLocker.
+///
+/// NOTE: All implementations of InterprocessSync should keep the
+/// is_locked_ member variable updated whenever their
+/// lock()/tryLock()/unlock() implementations are called.
class InterprocessSync {
// InterprocessSyncLocker is the only code outside this class that
// should be allowed to call the lock(), tryLock() and unlock()
@@ -100,7 +104,8 @@ public:
/// \brief Destructor
~InterprocessSyncLocker() {
- unlock();
+ if (isLocked())
+ unlock();
}
/// \brief Acquire the lock (blocks if something else has acquired a
@@ -113,11 +118,20 @@ public:
/// \brief Try to acquire a lock (doesn't block)
///
- /// \return Returns true if the lock was acquired, false otherwise.
+ /// \return Returns true if a new lock could be acquired, false
+ /// otherwise.
bool tryLock() {
return (sync_.tryLock());
}
+ /// \brief Check if the lock is taken
+ ///
+ /// \return Returns true if a lock is currently acquired, false
+ /// otherwise.
+ bool isLocked() {
+ return (sync_.is_locked_);
+ }
+
/// \brief Release the lock
///
/// \return Returns true if the lock was released, false otherwise.
diff --git a/src/lib/util/interprocess_sync_file.h b/src/lib/util/interprocess_sync_file.h
index 0c99929..fd8da1b 100644
--- a/src/lib/util/interprocess_sync_file.h
+++ b/src/lib/util/interprocess_sync_file.h
@@ -15,8 +15,8 @@
#ifndef __INTERPROCESS_SYNC_FILE_H__
#define __INTERPROCESS_SYNC_FILE_H__
-#include "util/interprocess_sync.h"
-#include "exceptions/exceptions.h"
+#include <util/interprocess_sync.h>
+#include <exceptions/exceptions.h>
namespace isc {
namespace util {
diff --git a/src/lib/util/interprocess_sync_null.cc b/src/lib/util/interprocess_sync_null.cc
index e2cfc9b..5355d57 100644
--- a/src/lib/util/interprocess_sync_null.cc
+++ b/src/lib/util/interprocess_sync_null.cc
@@ -22,16 +22,19 @@ InterprocessSyncNull::~InterprocessSyncNull() {
bool
InterprocessSyncNull::lock() {
+ is_locked_ = true;
return (true);
}
bool
InterprocessSyncNull::tryLock() {
+ is_locked_ = true;
return (true);
}
bool
InterprocessSyncNull::unlock() {
+ is_locked_ = false;
return (true);
}
diff --git a/src/lib/util/interprocess_sync_null.h b/src/lib/util/interprocess_sync_null.h
index 150d585..6ac0322 100644
--- a/src/lib/util/interprocess_sync_null.h
+++ b/src/lib/util/interprocess_sync_null.h
@@ -15,7 +15,7 @@
#ifndef __INTERPROCESS_SYNC_NULL_H__
#define __INTERPROCESS_SYNC_NULL_H__
-#include "util/interprocess_sync.h"
+#include <util/interprocess_sync.h>
namespace isc {
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 3b15037..a0b4109 100644
--- a/src/lib/util/tests/interprocess_sync_file_unittest.cc
+++ b/src/lib/util/tests/interprocess_sync_file_unittest.cc
@@ -55,7 +55,9 @@ TEST(InterprocessSyncFileTest, TestLock) {
InterprocessSyncFile sync("test");
InterprocessSyncLocker locker(sync);
+ EXPECT_FALSE(locker.isLocked());
EXPECT_TRUE(locker.lock());
+ EXPECT_TRUE(locker.isLocked());
int fds[2];
@@ -77,7 +79,10 @@ TEST(InterprocessSyncFileTest, TestLock) {
InterprocessSyncLocker locker2(sync2);
if (!locker2.tryLock()) {
+ EXPECT_FALSE(locker2.isLocked());
locked = 1;
+ } else {
+ EXPECT_TRUE(locker2.isLocked());
}
write(fds[1], &locked, sizeof(locked));
@@ -95,8 +100,9 @@ TEST(InterprocessSyncFileTest, TestLock) {
}
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) {
@@ -112,8 +118,8 @@ TEST(InterprocessSyncFileTest, TestMultipleFilesDirect) {
EXPECT_TRUE(locker.unlock());
- 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 "/test1_lockfile"));
+ EXPECT_EQ (0, unlink(TEST_DATA_TOPBUILDDIR "/test2_lockfile"));
}
TEST(InterprocessSyncFileTest, TestMultipleFilesForked) {
@@ -154,8 +160,8 @@ TEST(InterprocessSyncFileTest, TestMultipleFilesForked) {
EXPECT_TRUE(locker.unlock());
- 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 "/test1_lockfile"));
+ EXPECT_EQ (0, unlink(TEST_DATA_TOPBUILDDIR "/test2_lockfile"));
}
}
diff --git a/src/lib/util/tests/interprocess_sync_null_unittest.cc b/src/lib/util/tests/interprocess_sync_null_unittest.cc
index 81cff76..70e2b07 100644
--- a/src/lib/util/tests/interprocess_sync_null_unittest.cc
+++ b/src/lib/util/tests/interprocess_sync_null_unittest.cc
@@ -24,28 +24,43 @@ TEST(InterprocessSyncNullTest, TestNull) {
InterprocessSyncNull sync("test1");
InterprocessSyncLocker locker(sync);
- // All of these must always return true
-
+ // Check if the is_locked_ flag is set correctly during lock().
+ EXPECT_FALSE(locker.isLocked());
EXPECT_TRUE(locker.lock());
+ EXPECT_TRUE(locker.isLocked());
+
+ // lock() must always return true (this is called 4 times, just an
+ // arbitrary number)
EXPECT_TRUE(locker.lock());
EXPECT_TRUE(locker.lock());
EXPECT_TRUE(locker.lock());
EXPECT_TRUE(locker.lock());
+ // Check if the is_locked_ flag is set correctly during unlock().
+ EXPECT_TRUE(locker.isLocked());
EXPECT_TRUE(locker.unlock());
+ EXPECT_FALSE(locker.isLocked());
+
+ // unlock() must always return true (this is called 4 times, just an
+ // arbitrary number)
EXPECT_TRUE(locker.unlock());
EXPECT_TRUE(locker.unlock());
EXPECT_TRUE(locker.unlock());
EXPECT_TRUE(locker.unlock());
+ // Check if the is_locked_ flag is set correctly during tryLock().
+ EXPECT_FALSE(locker.isLocked());
EXPECT_TRUE(locker.tryLock());
+ EXPECT_TRUE(locker.isLocked());
+
+ // tryLock() must always return true (this is called 4 times, just an
+ // arbitrary number)
EXPECT_TRUE(locker.tryLock());
EXPECT_TRUE(locker.tryLock());
EXPECT_TRUE(locker.tryLock());
EXPECT_TRUE(locker.tryLock());
- // Random order
-
+ // Random order (should all return true)
EXPECT_TRUE(locker.unlock());
EXPECT_TRUE(locker.lock());
EXPECT_TRUE(locker.tryLock());
More information about the bind10-changes
mailing list