BIND 10 trac2349, updated. b57f65e686286f211ef1b177f69a81fa5baf3021 [2349] Revert pimpl classes, make abstract base class
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Oct 16 13:54:57 UTC 2012
The branch, trac2349 has been updated
via b57f65e686286f211ef1b177f69a81fa5baf3021 (commit)
from 6920774dd67be1221c379e9d5161f0cc932fbd73 (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 b57f65e686286f211ef1b177f69a81fa5baf3021
Author: Jelte Jansen <jelte at isc.org>
Date: Tue Oct 16 15:54:11 2012 +0200
[2349] Revert pimpl classes, make abstract base class
saves a second dynamic allocation (and more importantly, one that couldn't be freed)
-----------------------------------------------------------------------
Summary of changes:
src/lib/resolve/recursive_query.cc | 81 ++++++--------------
src/lib/resolve/recursive_query.h | 71 ++++-------------
src/lib/resolve/tests/recursive_query_unittest.cc | 15 ++--
.../resolve/tests/recursive_query_unittest_2.cc | 2 +-
.../resolve/tests/recursive_query_unittest_3.cc | 2 +-
5 files changed, 51 insertions(+), 120 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/resolve/recursive_query.cc b/src/lib/resolve/recursive_query.cc
index 68b06b0..7eae6fe 100644
--- a/src/lib/resolve/recursive_query.cc
+++ b/src/lib/resolve/recursive_query.cc
@@ -158,6 +158,7 @@ RecursiveQuery::setRttRecorder(boost::shared_ptr<RttRecorder>& recorder) {
rtt_recorder_ = recorder;
}
+namespace {
typedef std::pair<std::string, uint16_t> addr_t;
/*
@@ -167,11 +168,11 @@ typedef std::pair<std::string, uint16_t> addr_t;
*
* Used by RecursiveQuery::sendQuery.
*/
-class RunningQuery::RunningQueryImpl : public IOFetch::Callback {
+class RunningQuery : public IOFetch::Callback, public AbstractRunningQuery {
class ResolverNSASCallback : public isc::nsas::AddressRequestCallback {
public:
- ResolverNSASCallback(RunningQueryImpl* rq) : rq_(rq) {}
+ ResolverNSASCallback(RunningQuery* rq) : rq_(rq) {}
void success(const isc::nsas::NameserverAddress& address) {
// Success callback, send query to found namesever
@@ -191,7 +192,7 @@ public:
}
private:
- RunningQueryImpl* rq_;
+ RunningQuery* rq_;
};
@@ -292,7 +293,7 @@ private:
// The moment in time we sent a query to the nameserver above.
struct timeval current_ns_qsent_time;
- // RunningQueryImpl deletes itself when it is done. In order for us
+ // RunningQuery deletes itself when it is done. In order for us
// to do this safely, we must make sure that there are no events
// that might call back to it. There are two types of events in
// this sense; the timers we set ourselves (lookup and client),
@@ -669,7 +670,7 @@ SERVFAIL:
}
public:
- RunningQueryImpl(IOService& io,
+ RunningQuery(IOService& io,
const Question& question,
MessagePtr answer_message,
std::pair<std::string, uint16_t>& test_server,
@@ -712,7 +713,7 @@ public:
lookup_timer.expires_from_now(
boost::posix_time::milliseconds(lookup_timeout));
++outstanding_events_;
- lookup_timer.async_wait(boost::bind(&RunningQueryImpl::lookupTimeout, this));
+ lookup_timer.async_wait(boost::bind(&RunningQuery::lookupTimeout, this));
}
// Setup the timer to send an answer (client_timeout)
@@ -720,12 +721,14 @@ public:
client_timer.expires_from_now(
boost::posix_time::milliseconds(client_timeout));
++outstanding_events_;
- client_timer.async_wait(boost::bind(&RunningQueryImpl::clientTimeout, this));
+ client_timer.async_wait(boost::bind(&RunningQuery::clientTimeout, this));
}
doLookup();
}
+ virtual ~RunningQuery() {};
+
// called if we have a lookup timeout; if our callback has
// not been called, call it now. Then stop.
void lookupTimeout() {
@@ -896,7 +899,7 @@ public:
}
};
-class ForwardQuery::ForwardQueryImpl : public IOFetch::Callback {
+class ForwardQuery : public IOFetch::Callback, public AbstractRunningQuery {
private:
// The io service to handle async calls
IOService& io_;
@@ -928,7 +931,7 @@ private:
asio::deadline_timer lookup_timer;
// Make FowardQuery deletes itself safely. for more information see
- // the comments of outstanding_events in RunningQueryImpl.
+ // the comments of outstanding_events in RunningQuery.
size_t outstanding_events_;
// If we have a client timeout, we call back with a failure message,
@@ -959,7 +962,7 @@ private:
}
public:
- ForwardQueryImpl(IOService& io,
+ ForwardQuery(IOService& io,
ConstMessagePtr query_message,
MessagePtr answer_message,
boost::shared_ptr<AddressVector> upstream,
@@ -983,7 +986,7 @@ public:
lookup_timer.expires_from_now(
boost::posix_time::milliseconds(lookup_timeout));
++outstanding_events_;
- lookup_timer.async_wait(boost::bind(&ForwardQueryImpl::lookupTimeout, this));
+ lookup_timer.async_wait(boost::bind(&ForwardQuery::lookupTimeout, this));
}
// Setup the timer to send an answer (client_timeout)
@@ -991,12 +994,14 @@ public:
client_timer.expires_from_now(
boost::posix_time::milliseconds(client_timeout));
++outstanding_events_;
- client_timer.async_wait(boost::bind(&ForwardQueryImpl::clientTimeout, this));
+ client_timer.async_wait(boost::bind(&ForwardQuery::clientTimeout, this));
}
send();
}
+ virtual ~ForwardQuery() {};
+
virtual void lookupTimeout() {
if (!callback_called_) {
makeSERVFAIL();
@@ -1072,7 +1077,9 @@ public:
}
};
-RunningQuery*
+}
+
+AbstractRunningQuery*
RecursiveQuery::resolve(const QuestionPtr& question,
const isc::resolve::ResolverInterface::CallbackPtr callback)
{
@@ -1125,7 +1132,7 @@ RecursiveQuery::resolve(const QuestionPtr& question,
return (NULL);
}
-RunningQuery*
+AbstractRunningQuery*
RecursiveQuery::resolve(const Question& question,
MessagePtr answer_message,
OutputBufferPtr buffer,
@@ -1187,7 +1194,7 @@ RecursiveQuery::resolve(const Question& question,
return (NULL);
}
-ForwardQuery*
+AbstractRunningQuery*
RecursiveQuery::forward(ConstMessagePtr query_message,
MessagePtr answer_message,
OutputBufferPtr buffer,
@@ -1213,47 +1220,9 @@ RecursiveQuery::forward(ConstMessagePtr query_message,
// everything throught without interpretation, except
// QID, port number. The response will not be cached.
// It will delete itself when it is done
- return new ForwardQuery(io, query_message, answer_message,
- upstream_, buffer, callback, query_timeout_,
- client_timeout_, lookup_timeout_);
-}
-
-ForwardQuery::ForwardQuery(IOService& io,
- ConstMessagePtr query_message,
- MessagePtr answer_message,
- boost::shared_ptr<AddressVector> upstream,
- OutputBufferPtr buffer,
- isc::resolve::ResolverInterface::CallbackPtr cb,
- int query_timeout, int client_timeout, int lookup_timeout) {
- fqi_ = new ForwardQueryImpl(io, query_message, answer_message,
- upstream, buffer, cb, query_timeout,
- client_timeout, lookup_timeout);
-}
-
-ForwardQuery::~ForwardQuery() {
- delete fqi_;
-}
-
-RunningQuery::RunningQuery(isc::asiolink::IOService& io,
- const isc::dns::Question question,
- isc::dns::MessagePtr answer_message,
- std::pair<std::string, uint16_t>& test_server,
- isc::util::OutputBufferPtr buffer,
- isc::resolve::ResolverInterface::CallbackPtr cb,
- int query_timeout, int client_timeout, int lookup_timeout,
- unsigned retries,
- isc::nsas::NameserverAddressStore& nsas,
- isc::cache::ResolverCache& cache,
- boost::shared_ptr<RttRecorder>& recorder)
-{
- rqi_ = new RunningQueryImpl(io, question, answer_message, test_server,
- buffer, cb, query_timeout, client_timeout,
- lookup_timeout, retries, nsas, cache,
- recorder);
-}
-
-RunningQuery::~RunningQuery() {
- delete rqi_;
+ return (new ForwardQuery(io, query_message, answer_message,
+ upstream_, buffer, callback, query_timeout_,
+ client_timeout_, lookup_timeout_));
}
} // namespace asiodns
diff --git a/src/lib/resolve/recursive_query.h b/src/lib/resolve/recursive_query.h
index a8eab8d..7cda837 100644
--- a/src/lib/resolve/recursive_query.h
+++ b/src/lib/resolve/recursive_query.h
@@ -54,63 +54,23 @@ private:
typedef std::vector<std::pair<std::string, uint16_t> > AddressVector;
-/// \brief A forwarded query
-///
-/// This class represents an active forwarded query object;
-/// i.e. an outstanding query to a different server when operating
-/// in forwarder mode.
-///
-/// It can not be instantiated directly, but is created by
-/// RecursiveQuery::forward().
-///
-/// Its only public method is its destructor, and that should in theory
-/// not be called either except in some unit tests. Instances should
-/// delete themselves when the query is finished.
-class ForwardQuery {
- friend class RecursiveQuery;
-private:
- class ForwardQueryImpl;
- ForwardQueryImpl* fqi_;
- ForwardQuery(isc::asiolink::IOService& io,
- isc::dns::ConstMessagePtr query_message,
- isc::dns::MessagePtr answer_message,
- boost::shared_ptr<AddressVector> upstream,
- isc::util::OutputBufferPtr buffer,
- isc::resolve::ResolverInterface::CallbackPtr cb,
- int query_timeout, int client_timeout, int lookup_timeout);
-public:
- ~ForwardQuery();
-};
-
/// \brief A Running query
///
-/// This class represents an active running query object;
-/// i.e. an outstanding query to an authoritative name server.
+/// This base class represents an active running query object;
+/// i.e. an outstanding query to an authoritative name server or
+/// upstream server (when running in forwarder mode).
///
/// It can not be instantiated directly, but is created by
-/// RecursiveQuery::resolve().
+/// RecursiveQuery::resolve() and RecursiveQuery::forward().
///
/// Its only public method is its destructor, and that should in theory
/// not be called either except in some unit tests. Instances should
/// delete themselves when the query is finished.
-class RunningQuery {
- friend class RecursiveQuery;
-private:
- class RunningQueryImpl;
- RunningQueryImpl* rqi_;
- RunningQuery(isc::asiolink::IOService& io,
- const isc::dns::Question question,
- isc::dns::MessagePtr answer_message,
- std::pair<std::string, uint16_t>& test_server,
- isc::util::OutputBufferPtr buffer,
- isc::resolve::ResolverInterface::CallbackPtr cb,
- int query_timeout, int client_timeout, int lookup_timeout,
- unsigned retries,
- isc::nsas::NameserverAddressStore& nsas,
- isc::cache::ResolverCache& cache,
- boost::shared_ptr<RttRecorder>& recorder);
+class AbstractRunningQuery {
+protected:
+ AbstractRunningQuery() {};
public:
- ~RunningQuery();
+ virtual ~AbstractRunningQuery() {};
};
/// \brief Recursive Query
@@ -186,7 +146,7 @@ public:
/// \param question The question being answered <qname/qclass/qtype>
/// \param callback Callback object. See
/// \c ResolverInterface::Callback for more information
- RunningQuery* resolve(const isc::dns::QuestionPtr& question,
+ AbstractRunningQuery* resolve(const isc::dns::QuestionPtr& question,
const isc::resolve::ResolverInterface::CallbackPtr callback);
@@ -202,13 +162,14 @@ public:
/// \param buffer An output buffer into which the intermediate responses will
/// be copied.
/// \param server A pointer to the \c DNSServer object handling the client
- /// \return A pointer to the active RunningQuery object created by this
- /// call (if any); this object should delete itself in normal
- /// circumstances, and can normally be ignored by the caller,
- /// but a pointer is returned for use-cases such as unit tests.
+ /// \return A pointer to the active AbstractRunningQuery object
+ /// created by this call (if any); this object should delete
+ /// itself in normal circumstances, and can normally be ignored
+ /// by the caller, but a pointer is returned for use-cases
+ /// such as unit tests.
/// Returns NULL if the data was found internally and no actual
/// query was sent.
- RunningQuery* resolve(const isc::dns::Question& question,
+ AbstractRunningQuery* resolve(const isc::dns::Question& question,
isc::dns::MessagePtr answer_message,
isc::util::OutputBufferPtr buffer,
DNSServer* server);
@@ -228,7 +189,7 @@ public:
/// this object should delete itself in normal circumstances,
/// and can normally be ignored by the caller, but a pointer
/// is returned for use-cases such as unit tests.
- ForwardQuery* forward(isc::dns::ConstMessagePtr query_message,
+ AbstractRunningQuery* forward(isc::dns::ConstMessagePtr query_message,
isc::dns::MessagePtr answer_message,
isc::util::OutputBufferPtr buffer,
DNSServer* server,
diff --git a/src/lib/resolve/tests/recursive_query_unittest.cc b/src/lib/resolve/tests/recursive_query_unittest.cc
index 9582ac1..7e7557b 100644
--- a/src/lib/resolve/tests/recursive_query_unittest.cc
+++ b/src/lib/resolve/tests/recursive_query_unittest.cc
@@ -510,12 +510,13 @@ protected:
vector<uint8_t> callback_data_;
ScopedSocket sock_;
boost::shared_ptr<isc::util::unittests::TestResolver> resolver_;
- RunningQuery* running_query_;
+ AbstractRunningQuery* running_query_;
};
RecursiveQueryTest::RecursiveQueryTest() :
dns_service_(NULL), callback_(NULL), callback_protocol_(0),
- callback_native_(-1), resolver_(new isc::util::unittests::TestResolver()), running_query_(NULL)
+ callback_native_(-1), resolver_(new isc::util::unittests::TestResolver()),
+ running_query_(NULL)
{
nsas_.reset(new isc::nsas::NameserverAddressStore(resolver_));
}
@@ -661,7 +662,7 @@ TEST_F(RecursiveQueryTest, forwarderSend) {
OutputBufferPtr buffer(new OutputBuffer(0));
MessagePtr answer(new Message(Message::RENDER));
- ForwardQuery* fq = rq.forward(query_message, answer, buffer, &server);
+ AbstractRunningQuery* fq = rq.forward(query_message, answer, buffer, &server);
char data[4096];
size_t size = sizeof(data);
@@ -759,7 +760,7 @@ TEST_F(RecursiveQueryTest, forwardQueryTimeout) {
isc::resolve::initResponseMessage(question, *query_message);
boost::shared_ptr<MockResolverCallback> callback(new MockResolverCallback(&server));
- ForwardQuery* fq = query.forward(query_message, answer, buffer, &server, callback);
+ AbstractRunningQuery* fq = query.forward(query_message, answer, buffer, &server, callback);
// Run the test
io_service_.run();
EXPECT_EQ(callback->result, MockResolverCallback::FAILURE);
@@ -795,7 +796,7 @@ TEST_F(RecursiveQueryTest, forwardClientTimeout) {
isc::resolve::initResponseMessage(q, *query_message);
boost::shared_ptr<MockResolverCallback> callback(new MockResolverCallback(&server));
- ForwardQuery* fq = query.forward(query_message, answer, buffer, &server, callback);
+ AbstractRunningQuery* fq = query.forward(query_message, answer, buffer, &server, callback);
// Run the test
io_service_.run();
EXPECT_EQ(callback->result, MockResolverCallback::FAILURE);
@@ -832,7 +833,7 @@ TEST_F(RecursiveQueryTest, forwardLookupTimeout) {
isc::resolve::initResponseMessage(question, *query_message);
boost::shared_ptr<MockResolverCallback> callback(new MockResolverCallback(&server));
- ForwardQuery* fq = query.forward(query_message, answer, buffer, &server, callback);
+ AbstractRunningQuery* fq = query.forward(query_message, answer, buffer, &server, callback);
// Run the test
io_service_.run();
EXPECT_EQ(callback->result, MockResolverCallback::FAILURE);
@@ -868,7 +869,7 @@ TEST_F(RecursiveQueryTest, lowtimeouts) {
isc::resolve::initResponseMessage(question, *query_message);
boost::shared_ptr<MockResolverCallback> callback(new MockResolverCallback(&server));
- ForwardQuery* fq = query.forward(query_message, answer, buffer, &server, callback);
+ AbstractRunningQuery* fq = query.forward(query_message, answer, buffer, &server, callback);
// Run the test
io_service_.run();
EXPECT_EQ(callback->result, MockResolverCallback::FAILURE);
diff --git a/src/lib/resolve/tests/recursive_query_unittest_2.cc b/src/lib/resolve/tests/recursive_query_unittest_2.cc
index 134d9b1..40837a5 100644
--- a/src/lib/resolve/tests/recursive_query_unittest_2.cc
+++ b/src/lib/resolve/tests/recursive_query_unittest_2.cc
@@ -150,7 +150,7 @@ public:
udp::socket udp_socket_; ///< Socket used by UDP server
/// TODO: doc
- RunningQuery* running_query_;
+ AbstractRunningQuery* running_query_;
/// \brief Constructor
RecursiveQueryTest2() :
diff --git a/src/lib/resolve/tests/recursive_query_unittest_3.cc b/src/lib/resolve/tests/recursive_query_unittest_3.cc
index 6d200b0..3da2bff 100644
--- a/src/lib/resolve/tests/recursive_query_unittest_3.cc
+++ b/src/lib/resolve/tests/recursive_query_unittest_3.cc
@@ -133,7 +133,7 @@ public:
udp::socket udp_socket_; ///< Socket used by UDP server
/// TODO
- RunningQuery* running_query_;
+ AbstractRunningQuery* running_query_;
/// \brief Constructor
RecursiveQueryTest3() :
More information about the bind10-changes
mailing list