[2349] Same for RunningQuery

See 26483ea27726d4c1097485196a0908c565478188

src/lib/resolve/recursive_query.cc

@@ -158,8 +158,6 @@ RecursiveQuery::setRttRecorder(boost::shared_ptr<RttRecorder>& recorder) {
     rtt_recorder_ = recorder;
 }
 
-namespace {
-
 typedef std::pair<std::string, uint16_t> addr_t;
 
 /*
@@ -169,11 +167,11 @@ typedef std::pair<std::string, uint16_t> addr_t;
  *
  * Used by RecursiveQuery::sendQuery.
  */
-class RunningQuery : public IOFetch::Callback {
+class RunningQuery::RunningQueryImpl : public IOFetch::Callback {
 
 class ResolverNSASCallback : public isc::nsas::AddressRequestCallback {
 public:
-    ResolverNSASCallback(RunningQuery* rq) : rq_(rq) {}
+    ResolverNSASCallback(RunningQueryImpl* rq) : rq_(rq) {}
 
     void success(const isc::nsas::NameserverAddress& address) {
         // Success callback, send query to found namesever
@@ -193,7 +191,7 @@ public:
     }
 
 private:
-    RunningQuery* rq_;
+    RunningQueryImpl* rq_;
 };
 
 
@@ -294,7 +292,7 @@ private:
     // The moment in time we sent a query to the nameserver above.
     struct timeval current_ns_qsent_time;
 
-    // RunningQuery deletes itself when it is done. In order for us
+    // RunningQueryImpl 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),
@@ -671,7 +669,7 @@ SERVFAIL:
     }
 
 public:
-    RunningQuery(IOService& io,
+    RunningQueryImpl(IOService& io,
         const Question& question,
         MessagePtr answer_message,
         std::pair<std::string, uint16_t>& test_server,
@@ -714,7 +712,7 @@ public:
             lookup_timer.expires_from_now(
                 boost::posix_time::milliseconds(lookup_timeout));
             ++outstanding_events_;
-            lookup_timer.async_wait(boost::bind(&RunningQuery::lookupTimeout, this));
+            lookup_timer.async_wait(boost::bind(&RunningQueryImpl::lookupTimeout, this));
         }
 
         // Setup the timer to send an answer (client_timeout)
@@ -722,7 +720,7 @@ public:
             client_timer.expires_from_now(
                 boost::posix_time::milliseconds(client_timeout));
             ++outstanding_events_;
-            client_timer.async_wait(boost::bind(&RunningQuery::clientTimeout, this));
+            client_timer.async_wait(boost::bind(&RunningQueryImpl::clientTimeout, this));
         }
 
         doLookup();
@@ -892,12 +890,12 @@ public:
     // Clear the answer parts of answer_message, and set the rcode
     // to servfail
     void makeSERVFAIL() {
-        isc::resolve::makeErrorMessage(answer_message_, Rcode::SERVFAIL());
+        if (answer_message_) {
+            isc::resolve::makeErrorMessage(answer_message_, Rcode::SERVFAIL());
+        }
     }
 };
 
-}
-
 class ForwardQuery::ForwardQueryImpl : public IOFetch::Callback {
 private:
     // The io service to handle async calls
@@ -930,7 +928,7 @@ private:
     asio::deadline_timer lookup_timer;
 
     // Make FowardQuery deletes itself safely. for more information see
-    // the comments of outstanding_events in RunningQuery.
+    // the comments of outstanding_events in RunningQueryImpl.
     size_t outstanding_events_;
 
     // If we have a client timeout, we call back with a failure message,
@@ -1074,7 +1072,7 @@ public:
     }
 };
 
-void
+RunningQuery*
 RecursiveQuery::resolve(const QuestionPtr& question,
     const isc::resolve::ResolverInterface::CallbackPtr callback)
 {
@@ -1117,16 +1115,17 @@ RecursiveQuery::resolve(const QuestionPtr& question,
             // delete itself when it is done
             LOG_DEBUG(isc::resolve::logger, RESLIB_DBG_TRACE, RESLIB_RECQ_CACHE_NO_FIND)
                       .arg(questionText(*question)).arg(1);
-            new RunningQuery(io, *question, answer_message,
-                             test_server_, buffer, callback,
-                             query_timeout_, client_timeout_,
-                             lookup_timeout_, retries_, nsas_,
-                             cache_, rtt_recorder_);
+            return (new RunningQuery(io, *question, answer_message,
+                                     test_server_, buffer, callback,
+                                     query_timeout_, client_timeout_,
+                                     lookup_timeout_, retries_, nsas_,
+                                     cache_, rtt_recorder_));
         }
     }
+    return (NULL);
 }
 
-void
+RunningQuery*
 RecursiveQuery::resolve(const Question& question,
                         MessagePtr answer_message,
                         OutputBufferPtr buffer,
@@ -1179,12 +1178,13 @@ RecursiveQuery::resolve(const Question& question,
             // delete itself when it is done
             LOG_DEBUG(isc::resolve::logger, RESLIB_DBG_TRACE, RESLIB_RECQ_CACHE_NO_FIND)
                       .arg(questionText(question)).arg(2);
-            new RunningQuery(io, question, answer_message,
-                             test_server_, buffer, crs, query_timeout_,
-                             client_timeout_, lookup_timeout_, retries_,
-                             nsas_, cache_, rtt_recorder_);
+            return (new RunningQuery(io, question, answer_message,
+                                     test_server_, buffer, crs, query_timeout_,
+                                     client_timeout_, lookup_timeout_, retries_,
+                                     nsas_, cache_, rtt_recorder_));
         }
     }
+    return (NULL);
 }
 
 ForwardQuery*
@@ -1232,5 +1232,24 @@ 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_;
+}
+
 } // namespace asiodns
 } // namespace isc

src/lib/resolve/recursive_query.h

@@ -69,6 +69,25 @@ public:
     ~ForwardQuery();
 };
 
+class RunningQuery {
+private:
+    class RunningQueryImpl;
+    RunningQueryImpl* rqi_;
+public:
+    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);
+    ~RunningQuery();
+};
+
 /// \brief Recursive Query
 ///
 /// The \c RecursiveQuery class provides a layer of abstraction around
@@ -142,8 +161,8 @@ public:
     /// \param question The question being answered <qname/qclass/qtype>
     /// \param callback Callback object. See
     ///        \c ResolverInterface::Callback for more information
-    void resolve(const isc::dns::QuestionPtr& question,
-                 const isc::resolve::ResolverInterface::CallbackPtr callback);
+    RunningQuery* resolve(const isc::dns::QuestionPtr& question,
+        const isc::resolve::ResolverInterface::CallbackPtr callback);
 
 
     /// \brief Initiates resolving for the given question.
@@ -158,10 +177,10 @@ 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
-    void resolve(const isc::dns::Question& question,
-                 isc::dns::MessagePtr answer_message,
-                 isc::util::OutputBufferPtr buffer,
-                 DNSServer* server);
+    RunningQuery* resolve(const isc::dns::Question& question,
+                          isc::dns::MessagePtr answer_message,
+                          isc::util::OutputBufferPtr buffer,
+                          DNSServer* server);
 
     /// \brief Initiates forwarding for the given query.
     ///

src/lib/resolve/tests/recursive_query_unittest.cc

@@ -173,6 +173,9 @@ protected:
         // It would delete itself, but after the io_service_, which could
         // segfailt in case there were unhandled requests
         resolver_.reset();
+        // In a similar note, we wait until the resolver has been cleaned up
+        // until deleting and active test running_query_
+        delete running_query_;
     }
 
     void SetUp() {
@@ -507,11 +510,12 @@ protected:
     vector<uint8_t> callback_data_;
     ScopedSocket sock_;
     boost::shared_ptr<isc::util::unittests::TestResolver> resolver_;
+    RunningQuery* running_query_;
 };
 
 RecursiveQueryTest::RecursiveQueryTest() :
     dns_service_(NULL), callback_(NULL), callback_protocol_(0),
-    callback_native_(-1), resolver_(new isc::util::unittests::TestResolver())
+    callback_native_(-1), resolver_(new isc::util::unittests::TestResolver()), running_query_(NULL)
 {
     nsas_.reset(new isc::nsas::NameserverAddressStore(resolver_));
 }
@@ -833,7 +837,6 @@ TEST_F(RecursiveQueryTest, forwardLookupTimeout) {
     io_service_.run();
     EXPECT_EQ(callback->result, MockResolverCallback::FAILURE);
     delete fq;
-    std::cout << "[XX] DONE" << std::endl;
 }
 
 // Set everything very low and see if this doesn't cause weird
@@ -890,7 +893,7 @@ TEST_F(RecursiveQueryTest, DISABLED_recursiveSendOk) {
     Question q(Name("www.isc.org"), RRClass::IN(), RRType::A());
     OutputBufferPtr buffer(new OutputBuffer(0));
     MessagePtr answer(new Message(Message::RENDER));
-    rq.resolve(q, answer, buffer, &server);
+    running_query_ = rq.resolve(q, answer, buffer, &server);
     io_service_.run();
 
     // Check that the answer we got matches the one we wanted
@@ -916,7 +919,7 @@ TEST_F(RecursiveQueryTest, DISABLED_recursiveSendNXDOMAIN) {
     Question q(Name("wwwdoesnotexist.isc.org"), RRClass::IN(), RRType::A());
     OutputBufferPtr buffer(new OutputBuffer(0));
     MessagePtr answer(new Message(Message::RENDER));
-    rq.resolve(q, answer, buffer, &server);
+    running_query_ = rq.resolve(q, answer, buffer, &server);
     io_service_.run();
 
     // Check that the answer we got matches the one we wanted
@@ -980,9 +983,9 @@ TEST_F(RecursiveQueryTest, CachedNS) {
     // Prepare the recursive query
     vector<pair<string, uint16_t> > roots;
     roots.push_back(pair<string, uint16_t>("192.0.2.2", 53));
-
+    vector<pair<string, uint16_t> > upstream;
     RecursiveQuery rq(*dns_service_, *nsas_, cache_,
-                      vector<pair<string, uint16_t> >(), roots);
+                      upstream, roots);
     // Ask a question at the bottom. It should not use the lower NS, because
     // it would lead to a loop in NS. But it can use the nsUpper one, it has
     // an IP address and we can avoid asking root.
@@ -992,7 +995,7 @@ TEST_F(RecursiveQueryTest, CachedNS) {
     MessagePtr answer(new Message(Message::RENDER));
     // The server is here so we have something to pass there
     MockServer server(io_service_);
-    rq.resolve(q, answer, buffer, &server);
+    running_query_ = rq.resolve(q, answer, buffer, &server);
     // We don't need to run the service in this test. We are interested only
     // in the place it starts resolving at
 

src/lib/resolve/tests/recursive_query_unittest_2.cc

@@ -149,6 +149,9 @@ public:
     OutputBufferPtr udp_send_buffer_;           ///< Send buffer for UDP I/O
     udp::socket     udp_socket_;                ///< Socket used by UDP server
 
+    /// TODO: doc
+    RunningQuery* running_query_;
+
     /// \brief Constructor
     RecursiveQueryTest2() :
         debug_(DEBUG_PRINT),
@@ -170,8 +173,18 @@ public:
         udp_length_(0),
         udp_receive_buffer_(),
         udp_send_buffer_(new OutputBuffer(BUFFER_SIZE)),
-        udp_socket_(service_.get_io_service(), udp::v4())
-    {
+        udp_socket_(service_.get_io_service(), udp::v4()),
+        running_query_(NULL)
+    {}
+
+    ~RecursiveQueryTest2() {
+        // It would delete itself, but after the io_service_, which could
+        // segfailt in case there were unhandled requests
+        resolver_.reset();
+        // In a similar note, we wait until the resolver has been cleaned up
+        // until deleting and active test running_query_
+        delete running_query_;
+        delete nsas_;
     }
 
     /// \brief Set Common Message Bits
@@ -686,7 +699,7 @@ TEST_F(RecursiveQueryTest2, Resolve) {
     // Kick off the resolution process.  We expect the first question to go to
     // "root".
     expected_ = UDP_ROOT;
-    query.resolve(question_, resolver_callback);
+    running_query_ = query.resolve(question_, resolver_callback);
     service_.run();
 
     // Check what ran. (We have to cast the callback to ResolverCallback as we

src/lib/resolve/tests/recursive_query_unittest_3.cc

@@ -132,6 +132,9 @@ public:
     OutputBufferPtr udp_send_buffer_;           ///< Send buffer for UDP I/O
     udp::socket     udp_socket_;                ///< Socket used by UDP server
 
+    /// TODO
+    RunningQuery* running_query_;
+
     /// \brief Constructor
     RecursiveQueryTest3() :
         service_(),
@@ -154,10 +157,21 @@ public:
         udp_length_(0),
         udp_receive_buffer_(),
         udp_send_buffer_(new OutputBuffer(BUFFER_SIZE)),
-        udp_socket_(service_.get_io_service(), udp::v4())
+        udp_socket_(service_.get_io_service(), udp::v4()),
+        running_query_(NULL)
     {
     }
 
+    ~RecursiveQueryTest3() {
+        delete nsas_;
+        // It would delete itself, but after the io_service_, which could
+        // segfailt in case there were unhandled requests
+        resolver_.reset();
+        // In a similar note, we wait until the resolver has been cleaned up
+        // until deleting and active test running_query_
+        delete running_query_;
+    }
+
     /// \brief Set Common Message Bits
     ///
     /// Sets up the common bits of a response message returned by the handlers.
@@ -542,7 +556,7 @@ TEST_F(RecursiveQueryTest3, Resolve) {
 
     // Kick off the resolution process.
     expected_ = EDNS_UDP;
-    query.resolve(question_, resolver_callback);
+    running_query_ = query.resolve(question_, resolver_callback);
     service_.run();
 
     // Check what ran. (We have to cast the callback to ResolverCallback3 as we