Merge branch 'trac2935'

src/bin/auth/auth_srv.cc

@@ -375,28 +375,11 @@ public:
     {}
 };
 
-// This is a derived class of \c SimpleCallback, to serve
-// as a callback in the asiolink module.  It checks for queued
-// configuration messages, and executes them if found.
-class ConfigChecker : public SimpleCallback {
-public:
-    ConfigChecker(AuthSrv* srv) : server_(srv) {}
-    virtual void operator()(const IOMessage&) const {
-        ModuleCCSession* cfg_session = server_->getConfigSession();
-        if (cfg_session != NULL && cfg_session->hasQueuedMsgs()) {
-            cfg_session->checkCommand();
-        }
-    }
-private:
-    AuthSrv* server_;
-};
-
 AuthSrv::AuthSrv(isc::xfr::AbstractXfroutClient& xfrout_client,
                  isc::util::io::BaseSocketSessionForwarder& ddns_forwarder) :
     dnss_(NULL)
 {
     impl_ = new AuthSrvImpl(xfrout_client, ddns_forwarder);
-    checkin_ = new ConfigChecker(this);
     dns_lookup_ = new MessageLookup(this);
     dns_answer_ = new MessageAnswer(this);
 }
@@ -408,7 +391,6 @@ AuthSrv::stop() {
 
 AuthSrv::~AuthSrv() {
     delete impl_;
-    delete checkin_;
     delete dns_lookup_;
     delete dns_answer_;
 }

src/bin/auth/auth_srv.h

@@ -190,9 +190,6 @@ public:
     /// \brief Return pointer to the DNS Answer callback function
     isc::asiodns::DNSAnswer* getDNSAnswerProvider() const { return (dns_answer_); }
 
-    /// \brief Return pointer to the Checkin callback function
-    isc::asiolink::SimpleCallback* getCheckinProvider() const { return (checkin_); }
-
     /// \brief Return data source clients manager.
     ///
     /// \throw None
@@ -286,7 +283,6 @@ private:
     void foreignCommand(const std::string& command, const std::string&,
                         const isc::data::ConstElementPtr& params);
     AuthSrvImpl* impl_;
-    isc::asiolink::SimpleCallback* checkin_;
     isc::asiodns::DNSLookup* dns_lookup_;
     isc::asiodns::DNSAnswer* dns_answer_;
     isc::asiodns::DNSServiceBase* dnss_;

src/bin/auth/main.cc

@@ -175,12 +175,11 @@ main(int argc, char* argv[]) {
         auth_server = auth_server_.get();
         LOG_INFO(auth_logger, AUTH_SERVER_CREATED);
 
-        SimpleCallback* checkin = auth_server->getCheckinProvider();
         IOService& io_service = auth_server->getIOService();
         DNSLookup* lookup = auth_server->getDNSLookupProvider();
         DNSAnswer* answer = auth_server->getDNSAnswerProvider();
 
-        DNSService dns_service(io_service, checkin, lookup, answer);
+        DNSService dns_service(io_service, lookup, answer);
         auth_server->setDNSService(dns_service);
         LOG_DEBUG(auth_logger, DBG_AUTH_START, AUTH_DNS_SERVICES_CREATED);
 

src/bin/resolver/main.cc

@@ -168,7 +168,6 @@ main(int argc, char* argv[]) {
         resolver = boost::shared_ptr<Resolver>(new Resolver());
         LOG_DEBUG(resolver_logger, RESOLVER_DBG_INIT, RESOLVER_CREATED);
 
-        SimpleCallback* checkin = resolver->getCheckinProvider();
         DNSLookup* lookup = resolver->getDNSLookupProvider();
         DNSAnswer* answer = resolver->getDNSAnswerProvider();
 
@@ -217,7 +216,7 @@ main(int argc, char* argv[]) {
         cache.update(root_a_rrset);
         cache.update(root_aaaa_rrset);
 
-        DNSService dns_service(io_service, checkin, lookup, answer);
+        DNSService dns_service(io_service, lookup, answer);
         resolver->setDNSService(dns_service);
         LOG_DEBUG(resolver_logger, RESOLVER_DBG_INIT, RESOLVER_SERVICE_CREATED);
 

src/bin/resolver/resolver.cc

@@ -338,25 +338,9 @@ public:
     }
 };
 
-// This is a derived class of \c SimpleCallback, to serve
-// as a callback in the asiolink module.  It checks for queued
-// configuration messages, and executes them if found.
-class ConfigCheck : public SimpleCallback {
-public:
-    ConfigCheck(Resolver* srv) : server_(srv) {}
-    virtual void operator()(const IOMessage&) const {
-        if (server_->getConfigSession()->hasQueuedMsgs()) {
-            server_->getConfigSession()->checkCommand();
-        }
-    }
-private:
-    Resolver* server_;
-};
-
 Resolver::Resolver() :
     impl_(new ResolverImpl()),
     dnss_(NULL),
-    checkin_(NULL),
     dns_lookup_(NULL),
     dns_answer_(new MessageAnswer),
     nsas_(NULL),
@@ -365,13 +349,11 @@ Resolver::Resolver() :
     // Operations referring to "this" must be done in the constructor body
     // (some compilers will issue warnings if "this" is referred to in the
     // initialization list).
-    checkin_ = new ConfigCheck(this);
     dns_lookup_ = new MessageLookup(this);
 }
 
 Resolver::~Resolver() {
     delete impl_;
-    delete checkin_;
     delete dns_lookup_;
     delete dns_answer_;
 }

src/bin/resolver/resolver.h

@@ -131,9 +131,6 @@ public:
     /// \brief Return pointer to the DNS Answer callback function
     isc::asiodns::DNSAnswer* getDNSAnswerProvider() { return (dns_answer_); }
 
-    /// \brief Return pointer to the Checkin callback function
-    isc::asiolink::SimpleCallback* getCheckinProvider() { return (checkin_); }
-
     /**
      * \brief Specify the list of upstream servers.
      *
@@ -259,7 +256,6 @@ public:
 private:
     ResolverImpl* impl_;
     isc::asiodns::DNSServiceBase* dnss_;
-    isc::asiolink::SimpleCallback* checkin_;
     isc::asiodns::DNSLookup* dns_lookup_;
     isc::asiodns::DNSAnswer* dns_answer_;
     isc::nsas::NameserverAddressStore* nsas_;

src/lib/asiodns/README

@@ -26,9 +26,6 @@ So, in simplified form, the behavior of a DNS Server is:
       if not parent:
         break
 
-    # This callback informs the caller that a packet has arrived, and
-    # gives it a chance to update configuration, etc
-    SimpleCallback(packet)
     YIELD answer = DNSLookup(packet, this)
     response = DNSAnswer(answer)
     YIELD send(response)
@@ -37,7 +34,7 @@ At each "YIELD" point, the coroutine initiates an asynchronous operation,
 then pauses and turns over control to some other task on the ASIO service
 queue.  When the operation completes, the coroutine resumes.
 
-DNSLookup, DNSAnswer and SimpleCallback define callback methods
+DNSLookup and DNSAnswer define callback methods
 used by a DNS Server to communicate with the module that called it.
 They are abstract-only classes whose concrete implementations
 are supplied by the calling module.

src/lib/asiodns/dns_service.cc

@@ -37,9 +37,9 @@ class DNSAnswer;
 
 class DNSServiceImpl {
 public:
-    DNSServiceImpl(IOService& io_service, SimpleCallback* checkin,
+    DNSServiceImpl(IOService& io_service,
                    DNSLookup* lookup, DNSAnswer* answer) :
-            io_service_(io_service), checkin_(checkin), lookup_(lookup),
+            io_service_(io_service), lookup_(lookup),
             answer_(answer), tcp_recv_timeout_(5000)
     {}
 
@@ -50,13 +50,12 @@ public:
     typedef boost::shared_ptr<TCPServer> TCPServerPtr;
     typedef boost::shared_ptr<DNSServer> DNSServerPtr;
     std::vector<DNSServerPtr> servers_;
-    SimpleCallback* checkin_;
     DNSLookup* lookup_;
     DNSAnswer* answer_;
     size_t tcp_recv_timeout_;
 
     template<class Ptr, class Server> void addServerFromFD(int fd, int af) {
-        Ptr server(new Server(io_service_.get_io_service(), fd, af, checkin_,
+        Ptr server(new Server(io_service_.get_io_service(), fd, af,
                               lookup_, answer_));
         startServer(server);
     }
@@ -88,9 +87,9 @@ private:
     }
 };
 
-DNSService::DNSService(IOService& io_service, SimpleCallback* checkin,
+DNSService::DNSService(IOService& io_service,
                        DNSLookup* lookup, DNSAnswer *answer) :
-    impl_(new DNSServiceImpl(io_service, checkin, lookup, answer)),
+    impl_(new DNSServiceImpl(io_service, lookup, answer)),
     io_service_(io_service)
 {
 }

src/lib/asiodns/dns_service.h

@@ -107,8 +107,8 @@ public:
 /// DNSService is the service that handles DNS queries and answers with
 /// a given IOService. This class is mainly intended to hold all the
 /// logic that is shared between the authoritative and the recursive
-/// server implementations. As such, it handles asio, including config
-/// updates (through the 'Checkinprovider'), and listening sockets.
+/// server implementations. As such, it handles asio and listening
+/// sockets.
 class DNSService : public DNSServiceBase {
     ///
     /// \name Constructors and Destructor
@@ -132,11 +132,9 @@ public:
     /// Use addServerTCPFromFD() or addServerUDPFromFD() to add some servers.
     ///
     /// \param io_service The IOService to work with
-    /// \param checkin Provider for cc-channel events (see \c SimpleCallback)
     /// \param lookup The lookup provider (see \c DNSLookup)
     /// \param answer The answer provider (see \c DNSAnswer)
     DNSService(asiolink::IOService& io_service,
-               isc::asiolink::SimpleCallback* checkin,
                DNSLookup* lookup, DNSAnswer* answer);
 
     /// \brief The destructor.

src/lib/asiodns/tcp_server.cc

@@ -47,11 +47,10 @@ namespace asiodns {
 ///
 /// The constructor
 TCPServer::TCPServer(io_service& io_service, int fd, int af,
-                     const SimpleCallback* checkin,
                      const DNSLookup* lookup,
                      const DNSAnswer* answer) :
     io_(io_service), done_(false),
-    checkin_callback_(checkin), lookup_callback_(lookup),
+    lookup_callback_(lookup),
     answer_callback_(answer)
 {
     if (af != AF_INET && af != AF_INET6) {
@@ -203,16 +202,6 @@ TCPServer::operator()(asio::error_code ec, size_t length) {
         io_message_.reset(new IOMessage(data_.get(), length, *iosock_,
                                         *peer_));
 
-        // Perform any necessary operations prior to processing the incoming
-        // packet (e.g., checking for queued configuration messages).
-        //
-        // (XXX: it may be a performance issue to have this called for
-        // every single incoming packet; we may wish to throttle it somehow
-        // in the future.)
-        if (checkin_callback_ != NULL) {
-            (*checkin_callback_)(*io_message_);
-        }
-
         // If we don't have a DNS Lookup provider, there's no point in
         // continuing; we exit the coroutine permanently.
         if (lookup_callback_ == NULL) {

src/lib/asiodns/tcp_server.h

@@ -41,14 +41,12 @@ public:
     /// \param io_service the asio::io_service to work with
     /// \param fd the file descriptor of opened TCP socket
     /// \param af address family of the socket, either AF_INET or AF_INET6
-    /// \param checkin the callbackprovider for non-DNS events
     /// \param lookup the callbackprovider for DNS lookup events
     /// \param answer the callbackprovider for DNS answer events
     /// \throw isc::InvalidParameter if af is neither AF_INET nor AF_INET6
     /// \throw isc::asiolink::IOError when a low-level error happens, like the
     ///     fd is not a valid descriptor or it can't be listened on.
     TCPServer(asio::io_service& io_service, int fd, int af,
-              const isc::asiolink::SimpleCallback* checkin = NULL,
               const DNSLookup* lookup = NULL, const DNSAnswer* answer = NULL);
 
     void operator()(asio::error_code ec = asio::error_code(),
@@ -125,7 +123,6 @@ private:
     bool done_;
 
     // Callback functions provided by the caller
-    const isc::asiolink::SimpleCallback* checkin_callback_;
     const DNSLookup* lookup_callback_;
     const DNSAnswer* answer_callback_;
 

src/lib/asiodns/tests/dns_server_unittest.cc

@@ -103,14 +103,6 @@ class ServerStopper {
         DNSServer* server_to_stop_;
 };
 
-// \brief no check logic at all,just provide a checkpoint to stop the server
-class DummyChecker : public SimpleCallback, public ServerStopper {
-    public:
-        virtual void operator()(const IOMessage&) const {
-            stopServer();
-        }
-};
-
 // \brief no lookup logic at all,just provide a checkpoint to stop the server
 class DummyLookup : public DNSLookup, public ServerStopper {
 public:
@@ -369,7 +361,6 @@ class DNSServerTestBase : public::testing::Test {
     protected:
         DNSServerTestBase() :
             server_address_(ip::address::from_string(server_ip)),
-            checker_(new DummyChecker()),
             lookup_(new DummyLookup()),
             sync_lookup_(new SyncDummyLookup()),
             answer_(new SimpleAnswer()),
@@ -390,7 +381,6 @@ class DNSServerTestBase : public::testing::Test {
             if (tcp_server_) {
                 tcp_server_->stop();
             }
-            delete checker_;
             delete lookup_;
             delete sync_lookup_;
             delete answer_;
@@ -436,7 +426,6 @@ class DNSServerTestBase : public::testing::Test {
 
         asio::io_service service;
         const ip::address server_address_;
-        DummyChecker* const checker_;
         DummyLookup* lookup_;     // we need to replace it in some cases
         SyncDummyLookup*  const sync_lookup_;
         SimpleAnswer* const answer_;
@@ -507,7 +496,7 @@ protected:
         this->tcp_server_ =
             boost::shared_ptr<TCPServer>(new TCPServer(
                                              this->service, fd_tcp, AF_INET6,
-                                             this->checker_, this->lookup_,
+                                             this->lookup_,
                                              this->answer_));
     }
 
@@ -516,7 +505,7 @@ protected:
     boost::shared_ptr<UDPServerClass> createServer(int fd, int af) {
         return (boost::shared_ptr<UDPServerClass>(
                     new UDPServerClass(this->service, fd, af,
-                                       this->checker_, this->lookup_,
+                                       this->lookup_,
                                        this->answer_)));
     }
 };
@@ -571,16 +560,6 @@ TYPED_TEST(DNSServerTest, stopUDPServerBeforeItStartServing) {
     EXPECT_TRUE(this->serverStopSucceed());
 }
 
-// Test whether udp server stopped successfully during message check.
-// This only works for non-sync server; SyncUDPServer doesn't use checkin
-// callback.
-TEST_F(AsyncServerTest, stopUDPServerDuringMessageCheck) {
-    this->testStopServerByStopper(*this->udp_server_, this->udp_client_,
-                                  this->checker_);
-    EXPECT_EQ(std::string(""), this->udp_client_->getReceivedData());
-    EXPECT_TRUE(this->serverStopSucceed());
-}
-
 // Test whether udp server stopped successfully during query lookup
 TYPED_TEST(DNSServerTest, stopUDPServerDuringQueryLookup) {
     this->testStopServerByStopper(*this->udp_server_, this->udp_client_,
@@ -665,14 +644,6 @@ TYPED_TEST(DNSServerTest, stopTCPServerBeforeItStartServing) {
 }
 
 
-// Test whether tcp server stopped successfully during message check
-TYPED_TEST(DNSServerTest, stopTCPServerDuringMessageCheck) {
-    this->testStopServerByStopper(*this->tcp_server_, this->tcp_client_,
-                                  this->checker_);
-    EXPECT_EQ(std::string(""), this->tcp_client_->getReceivedData());
-    EXPECT_TRUE(this->serverStopSucceed());
-}
-
 // Test whether tcp server stopped successfully during query lookup
 TYPED_TEST(DNSServerTest, stopTCPServerDuringQueryLookup) {
     this->testStopServerByStopper(*this->tcp_server_, this->tcp_client_,
@@ -709,7 +680,7 @@ TYPED_TEST(DNSServerTest, stopTCPServeMoreThanOnce) {
 TYPED_TEST(DNSServerTestBase, invalidFamily) {
     // We abuse DNSServerTestBase for this test, as we don't need the
     // initialization.
-    EXPECT_THROW(TCPServer(this->service, 0, AF_UNIX, this->checker_,
+    EXPECT_THROW(TCPServer(this->service, 0, AF_UNIX,
                            this->lookup_, this->answer_),
                  isc::InvalidParameter);
 }
@@ -728,10 +699,10 @@ TYPED_TEST(DNSServerTestBase, invalidTCPFD) {
      asio backend does fail as it tries to insert it right away, but
      not the others, maybe we could make it run this at last on epoll-based
      systems).
-    EXPECT_THROW(UDPServer(service, -1, AF_INET, checker_, lookup_,
+    EXPECT_THROW(UDPServer(service, -1, AF_INET, lookup_,
                            answer_), isc::asiolink::IOError);
     */
-    EXPECT_THROW(TCPServer(this->service, -1, AF_INET, this->checker_,
+    EXPECT_THROW(TCPServer(this->service, -1, AF_INET,
                            this->lookup_, this->answer_),
                  isc::asiolink::IOError);
 }

src/lib/asiodns/tests/dns_service_unittest.cc

@@ -81,7 +81,7 @@ protected:
     UDPDNSServiceTest() :
         first_buffer_(NULL), second_buffer_(NULL),
         lookup(&first_buffer_, &second_buffer_, io_service),
-        dns_service(io_service, NULL, &lookup, NULL),
+        dns_service(io_service, &lookup, NULL),
         client_socket(io_service.get_io_service(), asio::ip::udp::v6()),
         server_ep(asio::ip::address::from_string(TEST_IPV6_ADDR),
                   lexical_cast<uint16_t>(TEST_SERVER_PORT)),

src/lib/asiodns/udp_server.cc

@@ -57,9 +57,9 @@ struct UDPServer::Data {
      * first packet. But we do initialize the socket in here.
      */
     Data(io_service& io_service, const ip::address& addr, const uint16_t port,
-        SimpleCallback* checkin, DNSLookup* lookup, DNSAnswer* answer) :
+         DNSLookup* lookup, DNSAnswer* answer) :
         io_(io_service), bytes_(0), done_(false),
-        checkin_callback_(checkin),lookup_callback_(lookup),
+        lookup_callback_(lookup),
         answer_callback_(answer)
     {
         // We must use different instantiations for v4 and v6;
@@ -72,10 +72,10 @@ struct UDPServer::Data {
         }
         socket_->bind(udp::endpoint(addr, port));
     }
-    Data(io_service& io_service, int fd, int af, SimpleCallback* checkin,
+    Data(io_service& io_service, int fd, int af,
          DNSLookup* lookup, DNSAnswer* answer) :
          io_(io_service), bytes_(0), done_(false),
-         checkin_callback_(checkin),lookup_callback_(lookup),
+         lookup_callback_(lookup),
          answer_callback_(answer)
     {
         if (af != AF_INET && af != AF_INET6) {
@@ -102,7 +102,6 @@ struct UDPServer::Data {
      */
     Data(const Data& other) :
         io_(other.io_), socket_(other.socket_), bytes_(0), done_(false),
-        checkin_callback_(other.checkin_callback_),
         lookup_callback_(other.lookup_callback_),
         answer_callback_(other.answer_callback_)
     {
@@ -166,7 +165,6 @@ struct UDPServer::Data {
     bool done_;
 
     // Callback functions provided by the caller
-    const SimpleCallback* checkin_callback_;
     const DNSLookup* lookup_callback_;
     const DNSAnswer* answer_callback_;
 
@@ -179,9 +177,9 @@ struct UDPServer::Data {
 /// The constructor. It just creates new internal state object
 /// and lets it handle the initialization.
 UDPServer::UDPServer(io_service& io_service, int fd, int af,
-                     SimpleCallback* checkin, DNSLookup* lookup,
+                     DNSLookup* lookup,
                      DNSAnswer* answer) :
-    data_(new Data(io_service, fd, af, checkin, lookup, answer))
+    data_(new Data(io_service, fd, af, lookup, answer))
 { }
 
 /// The function operator is implemented with the "stackless coroutine"
@@ -260,15 +258,6 @@ UDPServer::operator()(asio::error_code ec, size_t length) {
         data_->io_message_.reset(new IOMessage(data_->data_.get(),
             data_->bytes_, *data_->iosock_, *data_->peer_));
 
-        // Perform any necessary operations prior to processing an incoming
-        // query (e.g., checking for queued configuration messages).
-        //
-        // (XXX: it may be a performance issue to check in for every single
-        // incoming query; we may wish to throttle this in the future.)
-        if (data_->checkin_callback_ != NULL) {
-            (*data_->checkin_callback_)(*data_->io_message_);
-        }
-
         // If we don't have a DNS Lookup provider, there's no point in
         // continuing; we exit the coroutine permanently.
         if (data_->lookup_callback_ == NULL) {

src/lib/asiodns/udp_server.h

@@ -43,14 +43,12 @@ public:
     /// \param io_service the asio::io_service to work with
     /// \param fd the file descriptor of opened UDP socket
     /// \param af address family, either AF_INET or AF_INET6
-    /// \param checkin the callbackprovider for non-DNS events
     /// \param lookup the callbackprovider for DNS lookup events
     /// \param answer the callbackprovider for DNS answer events
     /// \throw isc::InvalidParameter if af is neither AF_INET nor AF_INET6
     /// \throw isc::asiolink::IOError when a low-level error happens, like the
     ///     fd is not a valid descriptor.
     UDPServer(asio::io_service& io_service, int fd, int af,
-              isc::asiolink::SimpleCallback* checkin = NULL,
               DNSLookup* lookup = NULL, DNSAnswer* answer = NULL);
 
     /// \brief The function operator

src/lib/resolve/tests/recursive_query_unittest.cc

@@ -56,7 +56,6 @@
 #include <asiolink/io_service.h>
 #include <asiolink/io_message.h>
 #include <asiolink/io_error.h>
-#include <asiolink/simple_callback.h>
 
 using isc::UnitTestUtil;
 using namespace std;
@@ -333,8 +332,7 @@ protected:
     // Set up empty DNS Service
     // Set up an IO Service queue without any addresses
     void setDNSService() {
-        dns_service_.reset(new DNSService(io_service_, callback_.get(), NULL,
-                                          NULL));
+        dns_service_.reset(new DNSService(io_service_, callback_.get(), NULL));
     }
 
     // Run a simple server test, on either IPv4 or IPv6, and over either
@@ -478,10 +476,12 @@ protected:
     };
 
 private:
-    class ASIOCallBack : public SimpleCallback {
+    class ASIOCallBack : public DNSLookup {
     public:
         ASIOCallBack(RecursiveQueryTest* test_obj) : test_obj_(test_obj) {}
-        void operator()(const IOMessage& io_message) const {
+        void operator()(const IOMessage& io_message,
+                        isc::dns::MessagePtr, isc::dns::MessagePtr,
+                        isc::util::OutputBufferPtr, DNSServer*) const {
             test_obj_->callBack(io_message);
         }
     private:

src/lib/resolve/tests/recursive_query_unittest_2.cc

@@ -159,7 +159,7 @@ public:
     RecursiveQueryTest2() :
         debug_(DEBUG_PRINT),
         service_(),
-        dns_service_(service_, NULL, NULL, NULL),
+        dns_service_(service_, NULL, NULL),
         question_(new Question(Name("www.example.org"), RRClass::IN(), RRType::A())),
         last_(NONE),
         expected_(NONE),

src/lib/resolve/tests/recursive_query_unittest_3.cc

@@ -141,7 +141,7 @@ public:
     /// \brief Constructor
     RecursiveQueryTest3() :
         service_(),
-        dns_service_(service_, NULL, NULL, NULL),
+        dns_service_(service_, NULL, NULL),
         question_(new Question(Name("ednsfallback"),
                   RRClass::IN(), RRType::A())),
         last_(NONE),