BIND 10 master, updated. 7eaa1760f3ed6cd57cf1b8458516fa3d457371d7 Merge #2202
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Oct 2 19:37:53 UTC 2012
The branch, master has been updated
via 7eaa1760f3ed6cd57cf1b8458516fa3d457371d7 (commit)
via ff7d15330f2a74d983972efd1872d1af7d2cd958 (commit)
via fc7462103f1fb344bf91707a1e2de99534e237b9 (commit)
via 1ade0a0c0c8ead227aa7f8d374a1748de2535b15 (commit)
via a2bcf6e5b22df3c9bed473d342485548c60c8773 (commit)
via 408728e4512f08c8e50e3f627cc46eb6ad38a315 (commit)
via 1943079713265d5ad64762b107288b6e501c70cf (commit)
via 0ca39bfe35f856eb54ff77c50f5b2230ed20313b (commit)
via 52d245f139122e86c07a4336fbb2495bdb6499d9 (commit)
via 8bd4cd42e061d49d098cb1387b3614226a161f6d (commit)
via dd5bb15644a8e6d0df49d93ee8e982d2d0d463ea (commit)
via 936498565a5249df01ad44a808bb447952240c90 (commit)
via c74022e2d7acf6b77979e9c8b646d5da95419ed3 (commit)
via 310150445a17a5043cb5e93ce5ecaddd0336b196 (commit)
via c4e44c1a57326915e7291e982e2271b6574f73fc (commit)
via da7375d619b7ba63b77c8430f8d67799d8f26026 (commit)
via dcde0c0b983dd0ce788d7273f0cf7e02b37a711d (commit)
via 46c77cb971eeb52802fba8091529c9947bfda05c (commit)
via af3d7616a9f65bf3effb25d2432d0955dcb5d03b (commit)
via 6e3d1a58e4ec1857fc0264e8070ce67f569b7e42 (commit)
via 076e5cff040e4da53df8dd60cee37a06e4e557c9 (commit)
via ac98e5dbf31b54eb74917d03e53ac071e8c8c92e (commit)
via 022f55634bf4865a315797abafedfbe43d54603b (commit)
via beccbfe1acb5e6e309d5dee9cbc4461434091520 (commit)
via abd88f16af1295f2ff67f5e9593e61d1539ef6e4 (commit)
via a3e7cc6cf85981ed5a8a5e63589dfe1767b01e64 (commit)
via 382204ee795971bf6daeebb95f6134bab10ca18d (commit)
via e25a0b2202650ef749340a1993b427bf99d962e5 (commit)
via 7cd33029c63a0cd0889dba94709cf6cadaea48ab (commit)
via c0128e80648ec83fb9f4e3cbf3bb7bba2ef0bbb5 (commit)
via b52c3d66c1462da3733725d2baf40a21b8f521d6 (commit)
via 10a4833ef74f0cf9aebc3861aa69cae6a25ce1cb (commit)
via 35d60f15a86733bb3bc9db77b3d8c7713d83d70d (commit)
via ff7d2f5f77fab130de8db0d59ce48a483a0a8b45 (commit)
via 99767bbe7b2a278ffa88086c24dcd6363d2ed0e5 (commit)
via 3559ddcbe67e91941139c8c74ed77bb9be501dc4 (commit)
via 53ea5695fbd2a596ffda7c9b7bf0dd503b101462 (commit)
via ec9e57a32cefe7db94c2b8b8bebc782b8e872fa9 (commit)
via 88b2af9980fb431eb97dc72cb39214d7e1700ab7 (commit)
from b7bff77c9dfefe339a386afcaeeb79b4927cfda3 (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 7eaa1760f3ed6cd57cf1b8458516fa3d457371d7
Merge: b7bff77 ff7d153
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Tue Oct 2 21:21:48 2012 +0200
Merge #2202
The locking of client lists in auth server. This is to allow background
loading later on.
Conflicts:
src/bin/auth/auth_srv.cc
src/bin/auth/auth_srv.h
commit ff7d15330f2a74d983972efd1872d1af7d2cd958
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue Oct 2 10:43:35 2012 -0700
[2202] minor style fix: folded a long line.
commit fc7462103f1fb344bf91707a1e2de99534e237b9
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue Oct 2 16:53:56 2012 +0200
[2202] Change comment style
So it is consistent with the rest.
commit 1ade0a0c0c8ead227aa7f8d374a1748de2535b15
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Tue Oct 2 16:21:44 2012 +0200
[2202] Use simpler test for mutex
As Jinmei suggests, a double seems to be not atomic enough, so we can
use that. This is mostly his code, slightly modified (and comments
rewritten).
commit a2bcf6e5b22df3c9bed473d342485548c60c8773
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Tue Oct 2 10:53:10 2012 +0200
[2202] Comment catching of exceptions
commit 408728e4512f08c8e50e3f627cc46eb6ad38a315
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Tue Oct 2 10:43:09 2012 +0200
[2202] Assert instead of throw
So we don't throw from a destructor. This should never happen anyway.
commit 1943079713265d5ad64762b107288b6e501c70cf
Merge: dcde0c0 0ca39bf
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Mon Oct 1 14:34:17 2012 -0700
[2202] Merge branch 'trac2202' with the remote version.
fixing conflicts, apparently due to a missing push:
src/lib/util/threads/tests/lock_unittest.cc
commit 0ca39bfe35f856eb54ff77c50f5b2230ed20313b
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Mon Oct 1 16:20:33 2012 +0200
[2202] The vector must be a reference
Otherwise, each thread would get its own copy of it, messing with it
privately, not causing any need for locking.
commit 52d245f139122e86c07a4336fbb2495bdb6499d9
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Mon Oct 1 15:52:04 2012 +0200
[2202] Drop support for recursive mutexes
They are not needed currently.
commit 8bd4cd42e061d49d098cb1387b3614226a161f6d
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Mon Oct 1 15:44:02 2012 +0200
[2202] Don't release the lock until rendered
Since the message could contain some data from inside of the locked
lists/data sources, we need to keep the lock until we render it.
commit dd5bb15644a8e6d0df49d93ee8e982d2d0d463ea
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Mon Oct 1 15:37:38 2012 +0200
[2202] More detail in TODO comment
commit 936498565a5249df01ad44a808bb447952240c90
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Mon Oct 1 15:25:02 2012 +0200
[2202] Avoid delete in different thread
commit c74022e2d7acf6b77979e9c8b646d5da95419ed3
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Mon Oct 1 15:22:39 2012 +0200
Add a timeout in case something got stuck
commit 310150445a17a5043cb5e93ce5ecaddd0336b196
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Mon Oct 1 15:17:51 2012 +0200
[2202] Try throwing std::exception too
commit c4e44c1a57326915e7291e982e2271b6574f73fc
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Mon Oct 1 15:10:50 2012 +0200
[2202] Test improvements
* Reimplementing destroyLocked as death test
* Comments
* Includes to use < > instead of " "
* Don't use c99-isms.
commit da7375d619b7ba63b77c8430f8d67799d8f26026
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Mon Oct 1 14:24:51 2012 +0200
[2202] Minor improvements
* Documentation
* Exception texts
* Exception safety for a rare situation
commit dcde0c0b983dd0ce788d7273f0cf7e02b37a711d
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Fri Sep 28 15:10:32 2012 -0700
[2202] small style fixes
- long/short lines
- position of ++/--
- spacing
- namespace
commit 46c77cb971eeb52802fba8091529c9947bfda05c
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Fri Sep 28 13:03:07 2012 -0700
[2202] editorial: fixed a typo in a comment
commit af3d7616a9f65bf3effb25d2432d0955dcb5d03b
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Fri Sep 28 09:54:00 2012 -0700
[2202] added new util/threads directory to the doxygen input list.
commit 6e3d1a58e4ec1857fc0264e8070ce67f569b7e42
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed Sep 26 11:29:07 2012 +0200
[2202] Don't throw from destructors
The situations are very bad anyway, and that should not happen during
normal operation. So we assert instead.
commit 076e5cff040e4da53df8dd60cee37a06e4e557c9
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed Sep 26 11:20:40 2012 +0200
[2202] Cleanup: use better variable name
The previous looked like it should be boolean (there's something good
about lisp's P suffix).
commit ac98e5dbf31b54eb74917d03e53ac071e8c8c92e
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed Sep 26 11:16:51 2012 +0200
[2202] Don't use namespace std
Because of fear of possible future collisions.
commit 022f55634bf4865a315797abafedfbe43d54603b
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed Sep 26 11:11:04 2012 +0200
[2202] Minor cleanups
* Removed an unneeded friend declaration.
* Typo
commit beccbfe1acb5e6e309d5dee9cbc4461434091520
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Mon Sep 17 10:10:47 2012 -0700
[2202] some more minor editorial fix: spacing
commit abd88f16af1295f2ff67f5e9593e61d1539ef6e4
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Fri Sep 7 16:08:03 2012 +0900
[2202] some editorial/style fixes
- spacing
- new lines for readability
- const
- brace position for catch
- position of ++/--
commit a3e7cc6cf85981ed5a8a5e63589dfe1767b01e64
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed Sep 5 13:03:33 2012 +0200
[2202] Unify class-struct
It should be the same according to the standard, but Clang seems not to
agree with that.
commit 382204ee795971bf6daeebb95f6134bab10ca18d
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed Sep 5 12:57:20 2012 +0200
[2202] Don't use PTHREAD_MUTEX_ROBUST
It seems it isn't portable and it's not very useful check.
commit e25a0b2202650ef749340a1993b427bf99d962e5
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed Sep 5 10:43:51 2012 +0200
[2202] Check the mutex is locked when it should
And update tests so they lock.
commit 7cd33029c63a0cd0889dba94709cf6cadaea48ab
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed Sep 5 10:14:32 2012 +0200
[2202] Debug-only method Mutex::locked
It can be used to check the thing is locked.
commit c0128e80648ec83fb9f4e3cbf3bb7bba2ef0bbb5
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Tue Sep 4 12:37:35 2012 +0200
Disable broken test
The test may be broken by design. In such case, it should be removed.
But it is left there for now, maybe it's just broken implementation and
someone (the reviewer?) may have an idea how to fix it.
commit b52c3d66c1462da3733725d2baf40a21b8f521d6
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Tue Sep 4 12:30:00 2012 +0200
[2202] Lock whenever we use the client lists
Hopefully at all places.
Any idea how we can test this? That it is locked in all places and that
we didn't forget a place?
commit 10a4833ef74f0cf9aebc3861aa69cae6a25ce1cb
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Tue Sep 4 12:08:14 2012 +0200
[2202] Add a mutex to the AuthSrv
To be used when accessing the client lists.
commit 35d60f15a86733bb3bc9db77b3d8c7713d83d70d
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Tue Sep 4 11:15:42 2012 +0200
[2202] Implement the mutex
And fix the tests. Some of them were broken a lot, some of them only
little.
commit ff7d2f5f77fab130de8db0d59ce48a483a0a8b45
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Mon Sep 3 15:10:06 2012 +0200
[2202] Tests for mutex
commit 99767bbe7b2a278ffa88086c24dcd6363d2ed0e5
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Mon Sep 3 14:16:47 2012 +0200
[2202] Add forgotten test file
commit 3559ddcbe67e91941139c8c74ed77bb9be501dc4
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Mon Sep 3 14:13:14 2012 +0200
[2202] Implement Thread wrapper
commit 53ea5695fbd2a596ffda7c9b7bf0dd503b101462
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Mon Sep 3 13:04:12 2012 +0200
Tests for the Thread class
commit ec9e57a32cefe7db94c2b8b8bebc782b8e872fa9
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri Aug 31 13:55:33 2012 +0200
[2202] Minimal thread interface
To test the Mutexes, we'll need to run threads. So this is mostly to
support some tests for now, but it'll be needed later too.
commit 88b2af9980fb431eb97dc72cb39214d7e1700ab7
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri Aug 31 13:08:53 2012 +0200
[2202] Interface of Mutex wrapper
Created with minimalistic set of functionality. Created a new library
for it, there'll be more thread-related stuff here.
-----------------------------------------------------------------------
Summary of changes:
configure.ac | 2 +
doc/Doxyfile | 5 +-
src/bin/auth/Makefile.am | 1 +
src/bin/auth/auth_srv.cc | 28 ++++
src/bin/auth/auth_srv.h | 35 ++++
src/bin/auth/benchmarks/Makefile.am | 1 +
src/bin/auth/command.cc | 4 +
src/bin/auth/datasrc_configurator.h | 3 +
src/bin/auth/tests/Makefile.am | 1 +
src/bin/auth/tests/auth_srv_unittest.cc | 29 +++-
src/bin/auth/tests/command_unittest.cc | 76 +++++----
.../auth/tests/datasrc_configurator_unittest.cc | 5 +
src/lib/util/Makefile.am | 2 +-
src/lib/util/threads/Makefile.am | 12 ++
src/lib/util/threads/lock.cc | 138 ++++++++++++++++
src/lib/util/threads/lock.h | 128 +++++++++++++++
src/lib/util/threads/tests/Makefile.am | 36 ++++
src/lib/util/threads/tests/lock_unittest.cc | 116 +++++++++++++
src/lib/util/{ => threads}/tests/run_unittests.cc | 2 +-
src/lib/util/threads/tests/thread_unittest.cc | 98 +++++++++++
src/lib/util/threads/thread.cc | 172 ++++++++++++++++++++
src/lib/util/threads/thread.h | 112 +++++++++++++
22 files changed, 966 insertions(+), 40 deletions(-)
create mode 100644 src/lib/util/threads/Makefile.am
create mode 100644 src/lib/util/threads/lock.cc
create mode 100644 src/lib/util/threads/lock.h
create mode 100644 src/lib/util/threads/tests/Makefile.am
create mode 100644 src/lib/util/threads/tests/lock_unittest.cc
copy src/lib/util/{ => threads}/tests/run_unittests.cc (93%)
create mode 100644 src/lib/util/threads/tests/thread_unittest.cc
create mode 100644 src/lib/util/threads/thread.cc
create mode 100644 src/lib/util/threads/thread.h
-----------------------------------------------------------------------
diff --git a/configure.ac b/configure.ac
index b064480..c6a2f01 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1215,6 +1215,8 @@ AC_CONFIG_FILES([Makefile
src/lib/server_common/tests/Makefile
src/lib/util/Makefile
src/lib/util/io/Makefile
+ src/lib/util/threads/Makefile
+ src/lib/util/threads/tests/Makefile
src/lib/util/unittests/Makefile
src/lib/util/python/Makefile
src/lib/util/pyunittests/Makefile
diff --git a/doc/Doxyfile b/doc/Doxyfile
index 7e122e9..f6b9fa0 100644
--- a/doc/Doxyfile
+++ b/doc/Doxyfile
@@ -579,8 +579,9 @@ INPUT = ../src/lib/exceptions ../src/lib/cc \
../src/lib/log/compiler ../src/lib/asiolink/ ../src/lib/nsas \
../src/lib/testutils ../src/lib/cache ../src/lib/server_common/ \
../src/bin/sockcreator/ ../src/lib/util/ ../src/lib/util/io/ \
- ../src/lib/resolve ../src/lib/acl ../src/bin/dhcp6 ../src/lib/dhcp \
- ../src/bin/dhcp4 ../tests/tools/perfdhcp devel
+ ../src/lib/util/threads/ ../src/lib/resolve ../src/lib/acl \
+ ../src/bin/dhcp6 ../src/lib/dhcp ../src/bin/dhcp4 \
+ ../tests/tools/perfdhcp devel
# This tag can be used to specify the character encoding of the source files
# that doxygen parses. Internally doxygen uses the UTF-8 encoding, which is
diff --git a/src/bin/auth/Makefile.am b/src/bin/auth/Makefile.am
index 3efa9d3..6128a4c 100644
--- a/src/bin/auth/Makefile.am
+++ b/src/bin/auth/Makefile.am
@@ -74,6 +74,7 @@ b10_auth_LDADD += $(top_builddir)/src/lib/log/libb10-log.la
b10_auth_LDADD += $(top_builddir)/src/lib/xfr/libb10-xfr.la
b10_auth_LDADD += $(top_builddir)/src/lib/server_common/libb10-server-common.la
b10_auth_LDADD += $(top_builddir)/src/lib/statistics/libb10-statistics.la
+b10_auth_LDADD += $(top_builddir)/src/lib/util/threads/libb10-threads.la
b10_auth_LDADD += $(SQLITE_LIBS)
# TODO: config.h.in is wrong because doesn't honor pkgdatadir
diff --git a/src/bin/auth/auth_srv.cc b/src/bin/auth/auth_srv.cc
index 82a98a4..b870ae2 100644
--- a/src/bin/auth/auth_srv.cc
+++ b/src/bin/auth/auth_srv.cc
@@ -26,6 +26,7 @@
#include <exceptions/exceptions.h>
#include <util/buffer.h>
+#include <util/threads/lock.h>
#include <dns/edns.h>
#include <dns/exceptions.h>
@@ -272,6 +273,10 @@ public:
boost::shared_ptr<ConfigurableClientList> getClientList(const RRClass&
rrclass)
{
+ // TODO: Debug-build only check
+ if (!mutex_.locked()) {
+ isc_throw(isc::Unexpected, "Not locked!");
+ }
const std::map<RRClass, boost::shared_ptr<ConfigurableClientList> >::
const_iterator it(client_lists_.find(rrclass));
if (it == client_lists_.end()) {
@@ -309,6 +314,8 @@ public:
isc::dns::Message& message,
bool done);
+ mutable util::thread::Mutex mutex_;
+
private:
bool xfrout_connected_;
AbstractXfroutClient& xfrout_client_;
@@ -636,6 +643,10 @@ AuthSrvImpl::processNormalQuery(const IOMessage& io_message, Message& message,
local_edns->setUDPSize(AuthSrvImpl::DEFAULT_LOCAL_UDPSIZE);
message.setEDNS(local_edns);
}
+ // Lock the client lists and keep them under the lock until the processing
+ // and rendering is done (this is the same mutex as from
+ // AuthSrv::getClientListMutex()).
+ isc::util::thread::Mutex::Locker locker(mutex_);
try {
const ConstQuestionPtr question = *message.beginQuestion();
@@ -667,6 +678,8 @@ AuthSrvImpl::processNormalQuery(const IOMessage& io_message, Message& message,
LOG_DEBUG(auth_logger, DBG_AUTH_MESSAGES, AUTH_SEND_NORMAL_RESPONSE)
.arg(renderer_.getLength()).arg(message);
return (true);
+ // The message can contain some data from the locked resource. But outside
+ // this method, we touch only the RCode of it, so it should be safe.
}
bool
@@ -920,6 +933,11 @@ AuthSrv::destroyDDNSForwarder() {
void
AuthSrv::setClientList(const RRClass& rrclass,
const boost::shared_ptr<ConfigurableClientList>& list) {
+ // TODO: Debug-build only check
+ if (!impl_->mutex_.locked()) {
+ isc_throw(isc::Unexpected, "Not locked");
+ }
+
if (list) {
impl_->client_lists_[rrclass] = list;
} else {
@@ -933,6 +951,11 @@ AuthSrv::getClientList(const RRClass& rrclass) {
vector<RRClass>
AuthSrv::getClientListClasses() const {
+ // TODO: Debug-build only check
+ if (!impl_->mutex_.locked()) {
+ isc_throw(isc::Unexpected, "Not locked");
+ }
+
vector<RRClass> result;
for (std::map<RRClass, boost::shared_ptr<ConfigurableClientList> >::
const_iterator it(impl_->client_lists_.begin());
@@ -942,6 +965,11 @@ AuthSrv::getClientListClasses() const {
return (result);
}
+util::thread::Mutex&
+AuthSrv::getClientListMutex() const {
+ return (impl_->mutex_);
+}
+
void
AuthSrv::setTCPRecvTimeout(size_t timeout) {
dnss_->setTCPRecvTimeout(timeout);
diff --git a/src/bin/auth/auth_srv.h b/src/bin/auth/auth_srv.h
index e2ffd71..ee7bd52 100644
--- a/src/bin/auth/auth_srv.h
+++ b/src/bin/auth/auth_srv.h
@@ -40,6 +40,9 @@ namespace util {
namespace io {
class BaseSocketSessionForwarder;
}
+namespace thread {
+class Mutex;
+}
}
namespace datasrc {
class ConfigurableClientList;
@@ -319,6 +322,38 @@ public:
/// has been set by setClientList.
std::vector<isc::dns::RRClass> getClientListClasses() const;
+ /// \brief Return a mutex for the client lists.
+ ///
+ /// Background loading of data uses threads. Therefore we need to protect
+ /// the client lists by a mutex, so they don't change (or get destroyed)
+ /// during query processing. Get (and lock) this mutex whenever you do
+ /// something with the lists and keep it locked until you finish. This
+ /// is correct:
+ /// \code
+ /// {
+ /// Mutex::Locker locker(auth->getClientListMutex());
+ /// boost::shared_ptr<isc::datasrc::ConfigurableClientList>
+ /// list(auth->getClientList(RRClass::IN()));
+ /// // Do some processing here
+ /// }
+ /// \endcode
+ ///
+ /// But this is not (it releases the mutex too soon):
+ /// \code
+ /// boost::shared_ptr<isc::datasrc::ConfigurableClientList> list;
+ /// {
+ /// Mutex::Locker locker(auth->getClientListMutex());
+ /// list = auth->getClientList(RRClass::IN()));
+ /// }
+ /// // Do some processing here
+ /// \endcode
+ ///
+ /// \note This method is const even if you are allowed to modify
+ /// (lock) the mutex. It's because locking of the mutex is not really
+ /// a modification of the server object and it is needed to protect the
+ /// lists even on read-only operations.
+ isc::util::thread::Mutex& getClientListMutex() const;
+
/// \brief Sets the timeout for incoming TCP connections
///
/// Incoming TCP connections that have not sent their data
diff --git a/src/bin/auth/benchmarks/Makefile.am b/src/bin/auth/benchmarks/Makefile.am
index 948e718..48e552a 100644
--- a/src/bin/auth/benchmarks/Makefile.am
+++ b/src/bin/auth/benchmarks/Makefile.am
@@ -34,5 +34,6 @@ query_bench_LDADD += $(top_builddir)/src/lib/asiolink/libb10-asiolink.la
query_bench_LDADD += $(top_builddir)/src/lib/server_common/libb10-server-common.la
query_bench_LDADD += $(top_builddir)/src/lib/asiodns/libb10-asiodns.la
query_bench_LDADD += $(top_builddir)/src/lib/statistics/libb10-statistics.la
+query_bench_LDADD += $(top_builddir)/src/lib/util/threads/libb10-threads.la
query_bench_LDADD += $(SQLITE_LIBS)
diff --git a/src/bin/auth/command.cc b/src/bin/auth/command.cc
index 43b2422..448a31b 100644
--- a/src/bin/auth/command.cc
+++ b/src/bin/auth/command.cc
@@ -21,6 +21,7 @@
#include <config/ccsession.h>
#include <exceptions/exceptions.h>
#include <dns/rrclass.h>
+#include <util/threads/lock.h>
#include <string>
@@ -189,6 +190,9 @@ public:
}
Name origin(origin_elem->stringValue());
+ // We're going to work with the client lists. They may be used
+ // from a different thread too, protect them.
+ isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
const boost::shared_ptr<isc::datasrc::ConfigurableClientList>
list(server.getClientList(zone_class));
diff --git a/src/bin/auth/datasrc_configurator.h b/src/bin/auth/datasrc_configurator.h
index f305a0d..6b2f582 100644
--- a/src/bin/auth/datasrc_configurator.h
+++ b/src/bin/auth/datasrc_configurator.h
@@ -20,6 +20,7 @@
#include <datasrc/client_list.h>
#include <config/ccsession.h>
#include <cc/data.h>
+#include <util/threads/lock.h>
#include <set>
@@ -119,6 +120,8 @@ public:
isc_throw(isc::InvalidOperation,
"Can't reconfigure while not initialized by init()");
}
+ // Lock the client lists, we're going to manipulate them.
+ isc::util::thread::Mutex::Locker locker(server_->getClientListMutex());
typedef std::map<std::string, isc::data::ConstElementPtr> Map;
typedef std::pair<isc::dns::RRClass, ListPtr> RollbackPair;
typedef std::pair<isc::dns::RRClass, isc::data::ConstElementPtr>
diff --git a/src/bin/auth/tests/Makefile.am b/src/bin/auth/tests/Makefile.am
index 75e4a27..7c01a6b 100644
--- a/src/bin/auth/tests/Makefile.am
+++ b/src/bin/auth/tests/Makefile.am
@@ -73,6 +73,7 @@ run_unittests_LDADD += $(top_builddir)/src/lib/nsas/libb10-nsas.la
run_unittests_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
run_unittests_LDADD += $(top_builddir)/src/lib/statistics/libb10-statistics.la
run_unittests_LDADD += $(top_builddir)/src/lib/config/tests/libfake_session.la
+run_unittests_LDADD += $(top_builddir)/src/lib/util/threads/libb10-threads.la
run_unittests_LDADD += $(GTEST_LDADD)
run_unittests_LDADD += $(SQLITE_LIBS)
diff --git a/src/bin/auth/tests/auth_srv_unittest.cc b/src/bin/auth/tests/auth_srv_unittest.cc
index e7b4ca7..e6e3ef7 100644
--- a/src/bin/auth/tests/auth_srv_unittest.cc
+++ b/src/bin/auth/tests/auth_srv_unittest.cc
@@ -39,6 +39,7 @@
#include <auth/datasrc_configurator.h>
#include <util/unittests/mock_socketsession.h>
+#include <util/threads/lock.h>
#include <dns/tests/unittest_util.h>
#include <testutils/dnsmessage_test.h>
#include <testutils/srv_test.h>
@@ -1425,10 +1426,13 @@ TEST_F(AuthSrvTest,
{
// Set real inmem client to proxy
updateInMemory(&server, "example.", CONFIG_INMEMORY_EXAMPLE);
- boost::shared_ptr<isc::datasrc::ConfigurableClientList>
- list(new FakeList(server.getClientList(RRClass::IN()), THROW_NEVER,
- false));
- server.setClientList(RRClass::IN(), list);
+ {
+ isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
+ boost::shared_ptr<isc::datasrc::ConfigurableClientList>
+ list(new FakeList(server.getClientList(RRClass::IN()), THROW_NEVER,
+ false));
+ server.setClientList(RRClass::IN(), list);
+ }
createDataFromFile("nsec3query_nodnssec_fromWire.wire");
server.processMessage(*io_message, *parse_message, *response_obuffer,
@@ -1451,6 +1455,7 @@ setupThrow(AuthSrv* server, ThrowWhen throw_when, bool isc_exception,
{
updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
+ isc::util::thread::Mutex::Locker locker(server->getClientListMutex());
boost::shared_ptr<isc::datasrc::ConfigurableClientList>
list(new FakeList(server->getClientList(RRClass::IN()), throw_when,
isc_exception, rrset));
@@ -1763,6 +1768,10 @@ TEST_F(AuthSrvTest, DDNSForwardCreateDestroy) {
// Check the client list accessors
TEST_F(AuthSrvTest, clientList) {
+ // We need to lock the mutex to make the (get|set)ClientList happy.
+ // There's a debug-build only check in them to make sure everything
+ // locks them and we call them directly here.
+ isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
// The lists don't exist. Therefore, the list of RRClasses is empty.
// We also have no IN list.
EXPECT_TRUE(server.getClientListClasses().empty());
@@ -1793,4 +1802,16 @@ TEST_F(AuthSrvTest, clientList) {
EXPECT_EQ(list, server.getClientList(RRClass::IN()));
}
+// We just test the mutex can be locked (exactly once).
+TEST_F(AuthSrvTest, mutex) {
+ isc::util::thread::Mutex::Locker l1(server.getClientListMutex());
+ // TODO: Once we have non-debug build, this one will not work, since
+ // we currently use the fact that we can't lock twice from the same
+ // thread. In the non-debug mode, this would deadlock.
+ // Skip then.
+ EXPECT_THROW({
+ isc::util::thread::Mutex::Locker l2(server.getClientListMutex());
+ }, isc::InvalidOperation);
+}
+
}
diff --git a/src/bin/auth/tests/command_unittest.cc b/src/bin/auth/tests/command_unittest.cc
index a22725e..e57de93 100644
--- a/src/bin/auth/tests/command_unittest.cc
+++ b/src/bin/auth/tests/command_unittest.cc
@@ -174,6 +174,7 @@ TEST_F(AuthCommandTest, shutdownIncorrectPID) {
// zones, and checks the zones are correctly loaded.
void
zoneChecks(AuthSrv& server) {
+ isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
EXPECT_EQ(ZoneFinder::SUCCESS, server.getClientList(RRClass::IN())->
find(Name("ns.test1.example")).finder_->
find(Name("ns.test1.example"), RRType::A())->code);
@@ -214,6 +215,7 @@ configureZones(AuthSrv& server) {
void
newZoneChecks(AuthSrv& server) {
+ isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
EXPECT_EQ(ZoneFinder::SUCCESS, server.getClientList(RRClass::IN())->
find(Name("ns.test1.example")).finder_->
find(Name("ns.test1.example"), RRType::A())->code);
@@ -271,30 +273,33 @@ TEST_F(AuthCommandTest,
"}]}"));
DataSourceConfigurator::testReconfigure(&server_, config);
- // Check that the A record at www.example.org does not exist
- EXPECT_EQ(ZoneFinder::NXDOMAIN, server_.getClientList(RRClass::IN())->
- find(Name("example.org")).finder_->
- find(Name("www.example.org"), RRType::A())->code);
-
- // Add the record to the underlying sqlite database, by loading
- // it as a separate datasource, and updating it
- ConstElementPtr sql_cfg = Element::fromJSON("{ \"type\": \"sqlite3\","
- "\"database_file\": \""
- + test_db + "\"}");
- DataSourceClientContainer sql_ds("sqlite3", sql_cfg);
- ZoneUpdaterPtr sql_updater =
- sql_ds.getInstance().getUpdater(Name("example.org"), false);
- RRsetPtr rrset(new RRset(Name("www.example.org."), RRClass::IN(),
- RRType::A(), RRTTL(60)));
- rrset->addRdata(rdata::createRdata(rrset->getType(),
- rrset->getClass(),
- "192.0.2.1"));
- sql_updater->addRRset(*rrset);
- sql_updater->commit();
-
- EXPECT_EQ(ZoneFinder::NXDOMAIN, server_.getClientList(RRClass::IN())->
- find(Name("example.org")).finder_->
- find(Name("www.example.org"), RRType::A())->code);
+ {
+ isc::util::thread::Mutex::Locker locker(server_.getClientListMutex());
+ // Check that the A record at www.example.org does not exist
+ EXPECT_EQ(ZoneFinder::NXDOMAIN, server_.getClientList(RRClass::IN())->
+ find(Name("example.org")).finder_->
+ find(Name("www.example.org"), RRType::A())->code);
+
+ // Add the record to the underlying sqlite database, by loading
+ // it as a separate datasource, and updating it
+ ConstElementPtr sql_cfg = Element::fromJSON("{ \"type\": \"sqlite3\","
+ "\"database_file\": \""
+ + test_db + "\"}");
+ DataSourceClientContainer sql_ds("sqlite3", sql_cfg);
+ ZoneUpdaterPtr sql_updater =
+ sql_ds.getInstance().getUpdater(Name("example.org"), false);
+ RRsetPtr rrset(new RRset(Name("www.example.org."), RRClass::IN(),
+ RRType::A(), RRTTL(60)));
+ rrset->addRdata(rdata::createRdata(rrset->getType(),
+ rrset->getClass(),
+ "192.0.2.1"));
+ sql_updater->addRRset(*rrset);
+ sql_updater->commit();
+
+ EXPECT_EQ(ZoneFinder::NXDOMAIN, server_.getClientList(RRClass::IN())->
+ find(Name("example.org")).finder_->
+ find(Name("www.example.org"), RRType::A())->code);
+ }
// Now send the command to reload it
result_ = execAuthServerCommand(server_, "loadzone",
@@ -302,20 +307,26 @@ TEST_F(AuthCommandTest,
"{\"origin\": \"example.org\"}"));
checkAnswer(0, "Successful load");
- // And now it should be present too.
- EXPECT_EQ(ZoneFinder::SUCCESS, server_.getClientList(RRClass::IN())->
- find(Name("example.org")).finder_->
- find(Name("www.example.org"), RRType::A())->code);
+ {
+ isc::util::thread::Mutex::Locker locker(server_.getClientListMutex());
+ // And now it should be present too.
+ EXPECT_EQ(ZoneFinder::SUCCESS, server_.getClientList(RRClass::IN())->
+ find(Name("example.org")).finder_->
+ find(Name("www.example.org"), RRType::A())->code);
+ }
// Some error cases. First, the zone has no configuration. (note .com here)
result_ = execAuthServerCommand(server_, "loadzone",
Element::fromJSON("{\"origin\": \"example.com\"}"));
checkAnswer(1, "example.com");
- // The previous zone is not hurt in any way
- EXPECT_EQ(ZoneFinder::SUCCESS, server_.getClientList(RRClass::IN())->
- find(Name("example.org")).finder_->
- find(Name("example.org"), RRType::SOA())->code);
+ {
+ isc::util::thread::Mutex::Locker locker(server_.getClientListMutex());
+ // The previous zone is not hurt in any way
+ EXPECT_EQ(ZoneFinder::SUCCESS, server_.getClientList(RRClass::IN())->
+ find(Name("example.org")).finder_->
+ find(Name("example.org"), RRType::SOA())->code);
+ }
const ConstElementPtr config2(Element::fromJSON("{"
"\"IN\": [{"
@@ -331,6 +342,7 @@ TEST_F(AuthCommandTest,
Element::fromJSON("{\"origin\": \"example.com\"}"));
checkAnswer(1, "Unreadable");
+ isc::util::thread::Mutex::Locker locker(server_.getClientListMutex());
// The previous zone is not hurt in any way
EXPECT_EQ(ZoneFinder::SUCCESS, server_.getClientList(RRClass::IN())->
find(Name("example.org")).finder_->
diff --git a/src/bin/auth/tests/datasrc_configurator_unittest.cc b/src/bin/auth/tests/datasrc_configurator_unittest.cc
index 78e3688..254b238 100644
--- a/src/bin/auth/tests/datasrc_configurator_unittest.cc
+++ b/src/bin/auth/tests/datasrc_configurator_unittest.cc
@@ -16,6 +16,7 @@
#include <config/tests/fake_session.h>
#include <config/ccsession.h>
+#include <util/threads/lock.h>
#include <gtest/gtest.h>
#include <memory>
@@ -81,6 +82,9 @@ public:
}
return (result);
}
+ isc::util::thread::Mutex& getClientListMutex() const {
+ return (mutex_);
+ }
protected:
DatasrcConfiguratorTest() :
session(ElementPtr(new ListElement), ElementPtr(new ListElement),
@@ -137,6 +141,7 @@ protected:
const string specfile;
std::map<RRClass, ListPtr> lists_;
string log_;
+ mutable isc::util::thread::Mutex mutex_;
};
// Check the initialization (and cleanup)
diff --git a/src/lib/util/Makefile.am b/src/lib/util/Makefile.am
index c56e4b8..fe1b327 100644
--- a/src/lib/util/Makefile.am
+++ b/src/lib/util/Makefile.am
@@ -1,4 +1,4 @@
-SUBDIRS = . io unittests tests pyunittests python
+SUBDIRS = . io unittests tests pyunittests python threads
AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
AM_CPPFLAGS += -I$(top_srcdir)/src/lib/util -I$(top_builddir)/src/lib/util
diff --git a/src/lib/util/threads/Makefile.am b/src/lib/util/threads/Makefile.am
new file mode 100644
index 0000000..85f70b9
--- /dev/null
+++ b/src/lib/util/threads/Makefile.am
@@ -0,0 +1,12 @@
+SUBDIRS = . tests
+AM_CXXFLAGS = $(B10_CXXFLAGS)
+
+AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
+AM_CPPFLAGS += $(BOOST_INCLUDES)
+
+lib_LTLIBRARIES = libb10-threads.la
+libb10_threads_la_SOURCES = lock.h lock.cc
+libb10_threads_la_SOURCES += thread.h thread.cc
+libb10_threads_la_LIBADD = $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
+
+CLEANFILES = *.gcno *.gcda
diff --git a/src/lib/util/threads/lock.cc b/src/lib/util/threads/lock.cc
new file mode 100644
index 0000000..de7bf07
--- /dev/null
+++ b/src/lib/util/threads/lock.cc
@@ -0,0 +1,138 @@
+// 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.
+
+#include "lock.h"
+
+#include <exceptions/exceptions.h>
+
+#include <cstring>
+#include <memory>
+#include <cerrno>
+#include <cassert>
+
+#include <pthread.h>
+
+using std::auto_ptr;
+
+namespace isc {
+namespace util {
+namespace thread {
+
+class Mutex::Impl {
+public:
+ Impl() :
+ locked_count(0)
+ {}
+ pthread_mutex_t mutex;
+ // Only in debug mode
+ size_t locked_count;
+};
+
+namespace {
+
+struct Deinitializer {
+ Deinitializer(pthread_mutexattr_t& attributes):
+ attributes_(attributes)
+ {}
+ ~Deinitializer() {
+ const int result = pthread_mutexattr_destroy(&attributes_);
+ // This should never happen. According to the man page,
+ // if there's error, it's our fault.
+ assert(result == 0);
+ }
+ pthread_mutexattr_t& attributes_;
+};
+
+}
+
+Mutex::Mutex() :
+ 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
+ result = pthread_mutexattr_settype(&attributes, PTHREAD_MUTEX_ERRORCHECK);
+ 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) {
+ const int result = pthread_mutex_destroy(&impl_->mutex);
+ const bool locked = impl_->locked_count != 0;
+ delete impl_;
+ // We don't want to throw from the destructor. Also, if this ever
+ // fails, something is really screwed up a lot.
+ assert(result == 0);
+
+ // We should not try to destroy a locked mutex, bad threaded monsters
+ // could get loose if we ever do and it is also forbidden by pthreads.
+
+ // This should not be possible to happen, since the
+ // pthread_mutex_destroy should check for it already. But it seems
+ // there are systems that don't check it.
+ assert(!locked);
+ }
+}
+
+void
+Mutex::lock() {
+ assert(impl_ != NULL);
+ const int result = pthread_mutex_lock(&impl_->mutex);
+ if (result != 0) {
+ isc_throw(isc::InvalidOperation, strerror(result));
+ }
+ ++impl_->locked_count; // Only in debug mode
+}
+
+void
+Mutex::unlock() {
+ assert(impl_ != NULL);
+ --impl_->locked_count; // Only in debug mode
+ const int result = pthread_mutex_unlock(&impl_->mutex);
+ assert(result == 0); // This should never be possible
+}
+
+// TODO: Disable in non-debug build
+bool
+Mutex::locked() const {
+ return (impl_->locked_count != 0);
+}
+
+}
+}
+}
diff --git a/src/lib/util/threads/lock.h b/src/lib/util/threads/lock.h
new file mode 100644
index 0000000..fef537b
--- /dev/null
+++ b/src/lib/util/threads/lock.h
@@ -0,0 +1,128 @@
+// 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.
+
+#ifndef B10_THREAD_LOCK_H
+#define B10_THREAD_LOCK_H
+
+#include <boost/noncopyable.hpp>
+
+#include <cstdlib> // for NULL.
+
+namespace isc {
+namespace util {
+namespace thread {
+
+/// \brief Mutex with very simple interface
+///
+/// Since mutexes are very system dependant, we create our own wrapper around
+/// whatever is available on the system and hide it.
+///
+/// To use this mutex, create it and then lock and unlock it by creating the
+/// Mutex::Locker object.
+///
+/// Also, as mutex is a low-level system object, an error might happen at any
+/// operation with it. We convert many errors to the isc::InvalidOperation,
+/// since the errors usually happen only when used in a wrong way. Any methods
+/// or constructors in this class can throw. Allocation errors are converted
+/// to std::bad_alloc (for example when OS-dependant limit of mutexes is
+/// exceeded). Some errors which usually mean a programmer error abort the
+/// program, since there could be no safe way to recover from them.
+///
+/// The current interface is somewhat minimalistic. If we ever need more, we
+/// can add it later.
+class Mutex : public boost::noncopyable {
+public:
+ /// \brief Constructor.
+ ///
+ /// Creates a mutex. It is a non-recursive mutex (can be locked just once,
+ /// if the same threads tries to lock it again, Bad Things Happen).
+ ///
+ /// Depending on compilation parameters and OS, the mutex may or may not
+ /// do some error and sanity checking. However, such checking is meant
+ /// only to aid development, not rely on it as a feature.
+ ///
+ /// \throw std::bad_alloc In case allocation of something (memory, the
+ /// OS mutex) fails.
+ /// \throw isc::InvalidOperation Other unspecified errors around the mutex.
+ /// This should be rare.
+ Mutex();
+
+ /// \brief Destructor.
+ ///
+ /// Destroys the mutex. It is not allowed to destroy a mutex which is
+ /// currently locked. This means a Locker created with this Mutex must
+ /// never live longer than the Mutex itself.
+ ~Mutex();
+
+ /// \brief This holds a lock on a Mutex.
+ ///
+ /// To lock a mutex, create a locker. It'll get unlocked when the locker
+ /// is destroyed.
+ ///
+ /// If you create the locker on the stack or using some other "garbage
+ /// collecting" mechanism (auto_ptr, for example), it ensures exception
+ /// safety with regards to the mutex - it'll get released on the exit
+ /// of function no matter by what means.
+ class Locker : public boost::noncopyable {
+ public:
+ /// \brief Constructor.
+ ///
+ /// Locks the mutex. May block for extended period of time.
+ ///
+ /// \throw isc::InvalidOperation when OS reports error. This usually
+ /// means an attempt to use the mutex in a wrong way (locking
+ /// a mutex second time from the same thread, for example).
+ Locker(Mutex& mutex) :
+ mutex_(NULL)
+ {
+ // Set the mutex_ after we acquire the lock. This is because of
+ // exception safety. If lock() throws, it didn't work, so we must
+ // not unlock when we are destroyed. In such case, mutex_ is
+ // NULL and checked in the destructor.
+ mutex.lock();
+ mutex_ = &mutex;
+ }
+
+ /// \brief Destructor.
+ ///
+ /// Unlocks the mutex.
+ ~Locker() {
+ if (mutex_ != NULL) {
+ mutex_->unlock();
+ }
+ }
+ private:
+ Mutex* mutex_;
+ };
+ /// \brief If the mutex is currently locked
+ ///
+ /// This is debug aiding method only. And it might be unavailable in
+ /// non-debug build (because keeping the state might be needlesly
+ /// slow).
+ ///
+ /// \todo Disable in non-debug build
+ bool locked() const;
+private:
+ class Impl;
+ Impl* impl_;
+ void lock();
+ void unlock();
+};
+
+
+}
+}
+}
+
+#endif
diff --git a/src/lib/util/threads/tests/Makefile.am b/src/lib/util/threads/tests/Makefile.am
new file mode 100644
index 0000000..e86aedc
--- /dev/null
+++ b/src/lib/util/threads/tests/Makefile.am
@@ -0,0 +1,36 @@
+SUBDIRS = .
+
+AM_CPPFLAGS = -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib
+AM_CPPFLAGS += $(BOOST_INCLUDES)
+# XXX: we'll pollute the top builddir for creating a temporary test file
+# # used to bind a UNIX domain socket so we can minimize the risk of exceeding
+# # the limit of file name path size.
+AM_CPPFLAGS += -DTEST_DATA_TOPBUILDDIR=\"$(abs_top_builddir)\"
+AM_CXXFLAGS = $(B10_CXXFLAGS)
+
+if USE_STATIC_LINK
+AM_LDFLAGS = -static
+endif
+
+CLEANFILES = *.gcno *.gcda
+
+TESTS_ENVIRONMENT = \
+ $(LIBTOOL) --mode=execute $(VALGRIND_COMMAND)
+
+TESTS =
+if HAVE_GTEST
+TESTS += run_unittests
+run_unittests_SOURCES = run_unittests.cc
+run_unittests_SOURCES += thread_unittest.cc
+run_unittests_SOURCES += lock_unittest.cc
+
+run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
+run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)
+
+run_unittests_LDADD = $(top_builddir)/src/lib/util/threads/libb10-threads.la
+run_unittests_LDADD += \
+ $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
+run_unittests_LDADD += $(GTEST_LDADD)
+endif
+
+noinst_PROGRAMS = $(TESTS)
diff --git a/src/lib/util/threads/tests/lock_unittest.cc b/src/lib/util/threads/tests/lock_unittest.cc
new file mode 100644
index 0000000..0b4d3ce
--- /dev/null
+++ b/src/lib/util/threads/tests/lock_unittest.cc
@@ -0,0 +1,116 @@
+// 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.
+
+#include <util/threads/lock.h>
+#include <util/threads/thread.h>
+
+#include <gtest/gtest.h>
+
+#include <boost/bind.hpp>
+#include <unistd.h>
+#include <signal.h>
+
+using namespace isc::util::thread;
+
+namespace {
+
+// If we try to lock the debug mutex multiple times, it should throw.
+TEST(MutexTest, lockMultiple) {
+ // TODO: Once we support non-debug mutexes, disable the test if we compile
+ // with them.
+ Mutex mutex;
+ EXPECT_FALSE(mutex.locked()); // Debug-only build
+ Mutex::Locker l1(mutex);
+ EXPECT_TRUE(mutex.locked()); // Debug-only build
+ EXPECT_THROW({
+ Mutex::Locker l2(mutex); // Attempt to lock again.
+ }, isc::InvalidOperation);
+ EXPECT_TRUE(mutex.locked()); // Debug-only build
+}
+
+// Destroying a locked mutex is a bad idea as well
+#ifdef EXPECT_DEATH
+TEST(MutexTest, destroyLocked) {
+ EXPECT_DEATH({
+ Mutex* mutex = new Mutex;
+ new Mutex::Locker(*mutex);
+ delete mutex;
+ // This'll leak the locker, but inside the slave process, it should
+ // not be an issue.
+ }, "");
+}
+#endif
+
+// In this test, we try to check if a mutex really locks. We could try that
+// with a deadlock, but that's not practical (the test would not end).
+//
+// Instead, we try do to some operation on the same data from multiple threads
+// that's likely to break if not locked. Also, the test must run for a while
+// to have an opportunity to manifest.
+//
+// Currently we try incrementing a double variable. That one is large enough
+// and complex enough so it should not be possible for the CPU to do it as an
+// atomic operation, at least on common architectures.
+const size_t iterations = 100000;
+
+void
+performIncrement(volatile double* canary, volatile bool* ready_me,
+ volatile bool* ready_other, Mutex* mutex)
+{
+ // Loosely (busy) wait for the other thread so both will start
+ // approximately at the same time.
+ *ready_me = true;
+ while (!*ready_other) {}
+
+ for (size_t i = 0; i < iterations; ++i) {
+ Mutex::Locker lock(*mutex);
+ *canary += 1;
+ }
+}
+
+void
+no_handler(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 = no_handler;
+ 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";
+ }
+}
+
+}
diff --git a/src/lib/util/threads/tests/run_unittests.cc b/src/lib/util/threads/tests/run_unittests.cc
new file mode 100644
index 0000000..6f71ad6
--- /dev/null
+++ b/src/lib/util/threads/tests/run_unittests.cc
@@ -0,0 +1,25 @@
+// 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.
+
+#include <gtest/gtest.h>
+#include <util/unittests/run_all.h>
+#include <stdlib.h>
+
+int
+main(int argc, char* argv[]) {
+ ::testing::InitGoogleTest(&argc, argv);
+
+ setenv("B10_LOCKFILE_DIR_FROM_BUILD", TEST_DATA_TOPBUILDDIR, 1);
+ return (isc::util::unittests::run_all());
+}
diff --git a/src/lib/util/threads/tests/thread_unittest.cc b/src/lib/util/threads/tests/thread_unittest.cc
new file mode 100644
index 0000000..5afdf3f
--- /dev/null
+++ b/src/lib/util/threads/tests/thread_unittest.cc
@@ -0,0 +1,98 @@
+// 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.
+
+#include <util/threads/thread.h>
+
+#include <boost/bind.hpp>
+
+#include <gtest/gtest.h>
+
+// This file tests the Thread class. It's hard to test an actual thread is
+// started, but we at least check the function is run and exceptions are
+// propagated as they should.
+//
+// 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 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).
+
+using namespace isc::util::thread;
+
+namespace {
+const size_t iterations = 200;
+const size_t detached_iterations = 25;
+
+void
+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));
+ }
+}
+
+void
+markRun(bool* mark) {
+ EXPECT_FALSE(*mark);
+ *mark = true;
+}
+
+// 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);
+ }
+}
+
+void
+throwSomething() {
+ throw 42; // Throw something really unusual, to see everything is caught.
+}
+
+void
+throwException() {
+ throw std::exception();
+}
+
+// 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);
+ }
+}
+
+// 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);
+ }
+}
+
+}
diff --git a/src/lib/util/threads/thread.cc b/src/lib/util/threads/thread.cc
new file mode 100644
index 0000000..3382fe2
--- /dev/null
+++ b/src/lib/util/threads/thread.cc
@@ -0,0 +1,172 @@
+// 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.
+
+#include "thread.h"
+#include "lock.h"
+
+#include <memory>
+#include <string>
+#include <cstring>
+#include <cerrno>
+
+#include <pthread.h>
+
+#include <boost/scoped_ptr.hpp>
+
+using std::string;
+using std::exception;
+using std::auto_ptr;
+using boost::scoped_ptr;
+
+namespace isc {
+namespace util {
+namespace thread {
+
+// The implementation of the Thread class.
+//
+// This internal state is not deleted until the thread terminates and is either
+// waited for or detached. We could do this with shared_ptr (or, shared_ptr and
+// weak_ptr), but we plan on compiling boost without thread support, so it
+// might not be safe. Therefore we use an explicit mutex. It is being locked
+// only 2-3 times in the lifetime of the thread, which should be negligible
+// overhead anyway.
+class Thread::Impl {
+public:
+ Impl(const boost::function<void ()>& main) :
+ // Two things to happen before destruction - thread needs to terminate
+ // and the creating thread needs to release it.
+ waiting_(2),
+ main_(main),
+ exception_(false)
+ {}
+ // Another of the waiting events is done. If there are no more, delete
+ // impl.
+ static void done(Impl* impl) {
+ bool should_delete(false);
+ { // We need to make sure the mutex is unlocked before it is deleted
+ Mutex::Locker locker(impl->mutex_);
+ if (--impl->waiting_ == 0) {
+ should_delete = true;
+ }
+ }
+ if (should_delete) {
+ delete impl;
+ }
+ }
+ // Run the thread. The type of parameter is because the pthread API.
+ static void* run(void* impl_raw) {
+ Impl* impl = static_cast<Impl*>(impl_raw);
+ try {
+ impl->main_();
+ } catch (const exception& e) {
+ impl->exception_ = true;
+ impl->exception_text_ = e.what();
+ } catch (...) {
+ impl->exception_ = true;
+ impl->exception_text_ = "Uknown exception";
+ }
+ done(impl);
+ return (NULL);
+ }
+ // How many events are waiting? One is for the thread to finish, one
+ // for the destructor of Thread or wait. Once both happen, this is
+ // no longer needed.
+ size_t waiting_;
+ // The main function of the thread.
+ boost::function<void ()> main_;
+ // Was there an exception?
+ bool exception_;
+ string exception_text_;
+ // The mutex protects the waiting_ member, which ensures there are
+ // no race conditions and collisions when terminating. The other members
+ // should be safe, because:
+ // * tid_ is read only.
+ // * exception_ and exception_text_ is accessed outside of the thread
+ // only after join, by that time the thread must have terminated.
+ // * main_ is used in a read-only way here. If there are any shared
+ // resources used inside, it is up to the main_ itself to take care.
+ Mutex mutex_;
+ // Which thread are we talking about anyway?
+ pthread_t tid_;
+};
+
+Thread::Thread(const boost::function<void ()>& main) :
+ impl_(NULL)
+{
+ auto_ptr<Impl> impl(new Impl(main));
+ const int result = pthread_create(&impl->tid_, NULL, &Impl::run,
+ impl.get());
+ // Any error here?
+ switch (result) {
+ case 0: // All 0K
+ impl_ = impl.release();
+ break;
+ case EAGAIN:
+ throw std::bad_alloc();
+ default: // Other errors. They should not happen.
+ isc_throw(isc::InvalidOperation, strerror(result));
+ }
+}
+
+Thread::~Thread() {
+ if (impl_ != NULL) {
+ // In case we didn't call wait yet
+ const int result = pthread_detach(impl_->tid_);
+ Impl::done(impl_);
+ impl_ = NULL;
+ // If the detach ever fails, something is screwed rather badly.
+ assert(result == 0);
+ }
+}
+
+void
+Thread::wait() {
+ if (impl_ == NULL) {
+ isc_throw(isc::InvalidOperation,
+ "Wait called and no thread to wait for");
+ }
+
+ const int result = pthread_join(impl_->tid_, NULL);
+ if (result != 0) {
+ isc_throw(isc::InvalidOperation, strerror(result));
+ }
+
+ // Was there an exception in the thread?
+ scoped_ptr<UncaughtException> ex;
+ // Something here could in theory throw. But we already terminated the thread, so
+ // we need to make sure we are in consistent state even in such situation (like
+ // releasing the mutex and impl_).
+ try {
+ if (impl_->exception_) {
+ ex.reset(new UncaughtException(__FILE__, __LINE__,
+ impl_->exception_text_.c_str()));
+ }
+ } catch (...) {
+ Impl::done(impl_);
+ impl_ = NULL;
+ // We have eaten the UncaughtException by now, but there's another
+ // exception instead, so we have at least something.
+ throw;
+ }
+
+ Impl::done(impl_);
+ impl_ = NULL;
+ if (ex.get() != NULL) {
+ throw UncaughtException(*ex);
+ }
+}
+
+}
+}
+}
diff --git a/src/lib/util/threads/thread.h b/src/lib/util/threads/thread.h
new file mode 100644
index 0000000..9c6d20a
--- /dev/null
+++ b/src/lib/util/threads/thread.h
@@ -0,0 +1,112 @@
+// 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.
+
+#ifndef B10_THREAD_H
+#define B10_THREAD_H
+
+#include <exceptions/exceptions.h>
+
+#include <boost/noncopyable.hpp>
+#include <boost/function.hpp>
+
+namespace isc {
+namespace util {
+/// \brief Wrappers for thread related functionality
+///
+/// We provide our own wrappers, currently around pthreads. We tried using
+/// the boost thread support, but it gave us some trouble, so we implemented
+/// in-house ones.
+namespace thread {
+
+/// \brief A separate thread.
+///
+/// A thread of execution. When created, starts running in the background.
+/// You can wait for it then or just forget it ever existed and leave it
+/// live peacefully.
+///
+/// The interface is minimalistic for now. We may need to extend it later.
+///
+/// \note While the objects of this class represent another thread, they
+/// are not thread-safe. You're not supposed to call wait() on the same
+/// object from multiple threads or so. They are reentrant (you can
+/// wait for different threads from different threads).
+class Thread : public boost::noncopyable {
+public:
+ /// \brief There's an uncaught exception in a thread.
+ ///
+ /// When a thread terminates because it the main function of the thread
+ /// throws, this one is re-thrown out of wait() and contains the what
+ /// of the original exception.
+ class UncaughtException : public isc::Exception {
+ public:
+ UncaughtException(const char* file, size_t line, const char* what) :
+ Exception(file, line, what)
+ {}
+ };
+
+ /// \brief Create and start a thread.
+ ///
+ /// Create a new thread and run body inside it.
+ ///
+ /// If you need to pass parameters to body, or return some result, you
+ /// may just want to use boost::bind or alike to store them within the
+ /// body functor.
+ ///
+ /// \note The main functor will be copied internally. You need to consider
+ /// this when returning the result.
+ ///
+ /// The body should terminate by exiting the function. If it throws, it
+ /// is considered an error. You should generally catch any exceptions form
+ /// within there and handle them somehow.
+ ///
+ /// \param main The code to run inside the thread.
+ ///
+ /// \throw std::bad_alloc if allocation of the new thread or other
+ /// resources fails.
+ /// \throw isc::InvalidOperation for other errors (should not happen).
+ Thread(const boost::function<void()>& main);
+
+ /// \brief Destructor.
+ ///
+ /// It is completely legitimate to destroy the thread without calling
+ /// wait() before. In such case, the thread will just live on until it
+ /// terminates. However, if the thread dies due to exception, for example,
+ /// it's up to you to detect that, no error is reported from this class.
+ ///
+ /// \throw isc::InvalidOperation in the rare case of OS reporting a
+ /// problem. This should not happen unless you messed up with the raw
+ /// thread by the low-level API.
+ ~Thread();
+
+ /// \brief Wait for the thread to terminate.
+ ///
+ /// Waits until the thread terminates. Must be called at most once.
+ ///
+ /// \throw isc::InvalidOperation if the OS API returns error. This usually
+ /// mean a programmer error (like two threads trying to wait on each
+ /// other).
+ /// \throw isc::InvalidOperation calling wait a second time.
+ /// \throw UncaughtException if the thread terminated by throwing an
+ /// exception instead of just returning from the function.
+ void wait();
+private:
+ class Impl;
+ Impl* impl_;
+};
+
+}
+}
+}
+
+#endif
More information about the bind10-changes
mailing list