NameserverEntry updated to newer interface

It now handles many things by itself and does its own locking.

Still one test does not pass currently, as it does not handle TTL.
There's a FIXME in the code, it allows wrong behaviour, but test for
that kind of behaviour isn't written yet.

git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac408@3630 e5f2f494-b856-4b98-b285-d166d9295462

src/lib/nsas/TODO

@@ -1,12 +1,6 @@
-* Make it aware of two address kinds, execute the callbacks if there is at
-  last one kind ready.
-
 Nameserver entries:
-* Upgrade the nameserver entries to own the callbacks (they will hold shared
-  pointers to zones). This will ensure the zone is not deleted at wrong time.
-* There's only one function for adding callbacks, put them together.
-* Return if we timed out.
-* When it expires, it should invalidate the zone up above it.
+* There's a big FIXME in askIP.
+* It has places where TTL should be handled, but isn't yet.
 
 Zone entries:
 * They will be nameserver callbacks themself. This ensures that they are not

src/lib/nsas/nameserver_entry.cc

@@ -18,6 +18,8 @@
 #include <functional>
 #include <cassert>
 #include <iostream>
+#include <boost/bind.hpp>
+#include <boost/foreach.hpp>
 
 #include <ctype.h>
 #include <strings.h>
@@ -47,7 +49,9 @@ namespace nsas {
 // Constructor, initialized with the list of addresses associated with this
 // nameserver.
 NameserverEntry::NameserverEntry(const AbstractRRset* v4Set,
-    const AbstractRRset* v6Set, time_t curtime) : expiration_(0)
+    const AbstractRRset* v6Set, time_t curtime) :
+    expiration_(0)
+
 {
     // TODO: Use pseudo-random RTT
     uint32_t rtt = 0;       // Round-trip time for an address
@@ -66,13 +70,16 @@ NameserverEntry::NameserverEntry(const AbstractRRset* v4Set,
     // random order).
 
 
+    has_address_[V4_ONLY] = has_address_[V6_ONLY] = false;
     // Do the V4 addresses first
     // XXX: Do we need to check that these are V4 addresses?
     if (v4Set) {
+        bool has_address(false);
         RdataIteratorPtr i = v4Set->getRdataIterator();
         // TODO Remove at merge with #410
         i->first();
         while (! i->isLast()) {
+            has_address = true;
             address_.push_back(AddressEntry(IOAddress(i->getCurrent().toText()),
             ++rtt));
             i->next();
@@ -82,15 +89,20 @@ NameserverEntry::NameserverEntry(const AbstractRRset* v4Set,
         expiration_ = curtime + v4Set->getTTL().getValue();
         v4name = v4Set->getName().toText(false);    // Ensure trailing dot
         v4class = v4Set->getClass().getCode();
+
+        // We have an address
+        has_address_[V4_ONLY] = has_address;
     }
 
     // Now the v6 addresses
     // XXX: Do we need to check that these are V6 addresses?
     if (v6Set) {
+        bool has_address(false);
         RdataIteratorPtr i = v6Set->getRdataIterator();
         // TODO Remove at merge with #410
         i->first();
         while (! i->isLast()) {
+            has_address = true;
             address_.push_back(AddressEntry(IOAddress(i->getCurrent().toText()),
             ++rtt));
             i->next();
@@ -108,6 +120,17 @@ NameserverEntry::NameserverEntry(const AbstractRRset* v4Set,
         // Extract the name of the v6 set and its class
         v6name = v6Set->getName().toText(false);    // Ensure trailing dot
         v6class = v6Set->getClass().getCode();
+
+        // We have an address
+        has_address_[V6_ONLY] = has_address;
+    }
+
+    // We got some addresses, so set we are ready & not expecting anything
+    if (has_address_[V4_ONLY] || has_address_[V6_ONLY]) {
+        has_address_[ANY_OK] = true;
+        expect_address_[ANY_OK] = expect_address_[V4_ONLY] =
+            expect_address_[V6_ONLY] = false;
+        setState(READY);
     }
 
     // TODO: Log a problem if both V4 and V6 address were null.
@@ -133,30 +156,106 @@ NameserverEntry::NameserverEntry(const AbstractRRset* v4Set,
     classCode_ = v4Set ? v4class : v6class;
 }
 
-// Returns the list of addresses matching the given family
-void NameserverEntry::getAddresses(AddressVector& addresses, short family) const {
-
-    // Quick check that allows validation of the assumption in the header file
-    // that a family value of 0 will select all address families.
-    assert(AF_INET != 0);
-    assert(AF_INET6 != 0);
-    
-    // Now copy all entries that meet the criteria.  Since remove_copy_if
-    // does the inverse (copies all entries that do not meet the criteria),
-    // the predicate for address selection is negated.
-    remove_copy_if(address_.begin(), address_.end(), back_inserter(addresses),
-        bind1st(AddressSelection(), family));
+namespace {
+
+/// Returns false if the address family of a given entry matches the address
+/// familyi given. This curious logic is needed for use in the remove_copy_if
+/// algorithm, which copies all values apart from those for which the
+/// criteria is met.
+bool
+addressSelection(short family, const AddressEntry& entry) {
+    return (entry.getAddress().getFamily() != family);
+};
+
+typedef mutex::scoped_lock Lock;
+
 }
 
-asiolink::IOAddress NameserverEntry::getAddressAtIndex(uint32_t index) const
+// Returns the list of addresses matching the given family
+Fetchable::State
+NameserverEntry::getAddresses(AddressVector& addresses,
+    AddressFamily family, bool expired_ok)
 {
+    Lock lock(mutex_);
+
+    /*
+     * FIXME: This switch is not completely correct. What if it is EXPIRED
+     * with expired_ok true and the given familly is unreachable?
+     *
+     * Write a test for this and:
+     * * First check if EXPIRED (and not expired_ok) or NOT_ASKED, then we are
+     *   done.
+     * * Second, check TTL and expire if not expired_ok. That might change us
+     *   to EXPIRED and we are done again.
+     * * Then see if we have the given address family ready and return.
+     * * Solve problem of both A and AAAA addresses with TTL 0 and then
+     *   when the second one comes, we should not give the first one even when
+     *   expired_ok, because it is in callback of the other one and it is
+     *   really expired.
+     */
+    switch (getState()) {
+        case IN_PROGRESS:
+            if (!has_address_[family] && expect_address_[family]) {
+                return IN_PROGRESS;
+            }
+            // If we do not expect the address, then fall trough to READY
+        case READY:
+            // TODO: Check TTL. If it is too old,
+            // set to EXPIRED and fall trough
+            // If we do not have this kind of address, it is unreachable by it
+            if (!has_address_[family]) {
+                return UNREACHABLE;
+            }
+            break;
+        case EXPIRED:
+            if (expired_ok) {
+                break;
+            }
+            // If expired is not OK, we just return state
+        case NOT_ASKED:
+        case UNREACHABLE:
+            // TODO: Check TTL of UNREACHABLE
+            // Reject giving any data
+            return (getState());
+    }
+
+    // If any address is OK, just pass everything we have
+    if (family == ANY_OK) {
+        addresses.insert(addresses.end(), address_.begin(), address_.end());
+    } else {
+        // Filter the addresses
+        short s_family(0);
+        switch (family) {
+            case V4_ONLY: s_family = AF_INET; break;
+            case V6_ONLY: s_family = AF_INET6; break;
+            default: assert(0); // This should never happen
+        }
+        // Now copy all entries that meet the criteria.  Since remove_copy_if
+        // does the inverse (copies all entries that do not meet the criteria),
+        // the predicate for address selection is negated.
+        remove_copy_if(address_.begin(), address_.end(),
+            back_inserter(addresses), boost::bind(addressSelection, s_family,
+            _1));
+    }
+    if (getState() == EXPIRED && expired_ok) {
+        return READY;
+    }
+    return getState();
+}
+
+asiolink::IOAddress
+NameserverEntry::getAddressAtIndex(uint32_t index) const {
+    Lock lock(mutex_);
+
     assert(index < address_.size());
 
     return address_[index].getAddress();
 }
 
 // Set the address RTT to a specific value
-void NameserverEntry::setAddressRTT(const IOAddress& address, uint32_t rtt) {
+void
+NameserverEntry::setAddressRTT(const IOAddress& address, uint32_t rtt) {
+    Lock lock(mutex_);
 
     // Search through the list of addresses for a match
     for (AddressVectorIterator i = address_.begin(); i != address_.end(); ++i) {
@@ -167,7 +266,10 @@ void NameserverEntry::setAddressRTT(const IOAddress& address, uint32_t rtt) {
 }
 
 // Update the address's rtt 
-void NameserverEntry::updateAddressRTTAtIndex(uint32_t rtt, uint32_t index) {
+void
+NameserverEntry::updateAddressRTTAtIndex(uint32_t rtt, uint32_t index) {
+    Lock lock(mutex_);
+
     //make sure it is a valid index
     if(index >= address_.size()) return;
 
@@ -176,121 +278,206 @@ void NameserverEntry::updateAddressRTTAtIndex(uint32_t rtt, uint32_t index) {
 }
 
 // Sets the address to be unreachable
-void NameserverEntry::setAddressUnreachable(const IOAddress& address) {
+void
+NameserverEntry::setAddressUnreachable(const IOAddress& address) {
     setAddressRTT(address, AddressEntry::UNREACHABLE);
 }
 
-void NameserverEntry::ensureHasCallback(shared_ptr<ZoneEntry> zone,
-    NameserverEntry::Callback& callback)
-{
-    if (getState() != Fetchable::IN_PROGRESS) {
-        isc_throw(BadValue,
-            "Callbacks can be added only to IN_PROGRESS nameserver entries");
-    }
-    if (ipCallbacks_.find(zone) == ipCallbacks_.end()) {
-        ipCallbacks_[zone] = &callback;
-    }
-}
-
+/*
+ * A callback class into the resolver. Whenever we ask the resolver something,
+ * this is created and the answer is fed back trough this. It holds a shared
+ * pointer to the entry so it is not destroyed too soon.
+ */
 class NameserverEntry::ResolverCallback : public ResolverInterface::Callback {
     public:
-        ResolverCallback(shared_ptr<NameserverEntry> entry) :
+        ResolverCallback(shared_ptr<NameserverEntry> entry,
+            AddressFamily family, const RRType& type) :
             entry_(entry),
-            rtt(0)
+            rtt_(0),
+            family_(family),
+            type_(type)
         { }
         virtual void success(const Message& response) {
-            map<shared_ptr<ZoneEntry>, NameserverEntry::Callback*> callbacks;
-            bool has_address(false);
+            Lock lock(entry_->mutex_);
+
+            vector<AddressEntry> entries;
+
+            bool found(false);
+            for (RRsetIterator set(
+                // TODO Trunk does Section::ANSWER() by constant
+                response.beginSection(Section::ANSWER()));
+                set != response.endSection(Section::ANSWER()); ++ set)
             {
-                mutex::scoped_lock lock(entry_->mutex_);
-                for (RRsetIterator set(
-                    // TODO Trunk does Section::ANSWER() by constant
-                    response.beginSection(Section::ANSWER()));
-                    set != response.endSection(Section::ANSWER()); ++ set)
+                if (found) {
+                    // TODO Log this
+                    // There are more than one RRset in the answer,
+                    // this shouldn't happen
+                    failureInternal(lock);
+                    return;
+                }
+
+                if ((*set)->getType() != type_ ||
+                    (*set)->getClass() != RRClass(entry_->getClass()))
                 {
-                    /**
-                     * TODO Move to common function, this is similar to
-                     * what is in constructor.
-                     */
-                    RdataIteratorPtr i((*set)->getRdataIterator());
-                    // TODO Remove at merge with #410
-                    i->first();
-                    while (! i->isLast()) {
-                        has_address = true;
-                        entry_->address_.push_back(AddressEntry(IOAddress(
-                        i->getCurrent().toText()), ++rtt));
-                        i->next();
-                    }
+                    // TODO Log we got answer of different type
+                    failureInternal(lock);
+                    return;
                 }
-                if (has_address) {
-                    callbacks.swap(entry_->ipCallbacks_);
-                    entry_->waiting_responses_ --;
-                    entry_->setState(Fetchable::READY);
+
+                /**
+                 * TODO Move to common function, this is similar to
+                 * what is in constructor.
+                 */
+                RdataIteratorPtr i((*set)->getRdataIterator());
+                // TODO Remove at merge with trunk
+                i->first();
+                while (! i->isLast()) {
+                    entries.push_back(AddressEntry(IOAddress(
+                        i->getCurrent().toText()), ++ rtt_));
+                    i->next();
                 }
-            } // Unlock
-            if (has_address) {
-                dispatchCallbacks(callbacks);
+            }
+
+            if (entries.empty()) {
+                // No data there, count it as a failure
+                failureInternal(lock);
             } else {
-                // No address there, so we take it as a failure
-                failure();
+                entry_->expect_address_[family_] = false;
+                entry_->expect_address_[ANY_OK] =
+                    entry_->expect_address_[V4_ONLY] ||
+                    entry_->expect_address_[V6_ONLY];
+                // Everything is here
+                if (!entry_->expect_address_[ANY_OK]) {
+                    entry_->setState(READY);
+                }
+                // We have some address
+                entry_->has_address_[ANY_OK] =
+                    entry_->has_address_[family_] = true;
+                // Put the addresses there
+                entry_->address_.insert(entry_->address_.end(),
+                    entries.begin(), entries.end());
+                // Run the right callbacks
+                dispatchCallbacks(lock);
             }
         }
         virtual void failure() {
-            map<shared_ptr<ZoneEntry>, NameserverEntry::Callback*> callbacks;
-            {
-                mutex::scoped_lock lock(entry_->mutex_);
-                entry_->waiting_responses_ --;
-                // Do we still have a chance to get the answer?
-                // Or did we already?
-                if (entry_->waiting_responses_ ||
-                    entry_->getState() != Fetchable::IN_PROGRESS)
-                {
-                    return;
-                }
-                // Remove the callbacks and call them now
-                callbacks.swap(entry_->ipCallbacks_);
-                entry_->setState(Fetchable::UNREACHABLE);
-            } // Unlock
-            dispatchCallbacks(callbacks);
+            Lock lock(entry_->mutex_);
+            failureInternal(lock);
         }
     private:
         shared_ptr<NameserverEntry> entry_;
-        int rtt;
-        void dispatchCallbacks(map<shared_ptr<ZoneEntry>,
-            NameserverEntry::Callback*>& callbacks)
+        int rtt_;
+        AddressFamily family_;
+        RRType type_;
+
+        // Dispatches all relevant callbacks. Keeps lock unlocked afterwards.
+        void dispatchCallbacks(Lock& lock)
         {
+            // We dispatch ANY addresses if there is at last one address or
+            // there's no chance we'll get some in future
+            bool dispatch_any = entry_->has_address_[ANY_OK] ||
+                !entry_->expect_address_[ANY_OK];
+            // Sort out the callbacks we want
+            vector<CallbackPair> keep;
+            vector<shared_ptr<NameserverEntry::Callback> > dispatch;
+            BOOST_FOREACH(const CallbackPair &callback, entry_->callbacks_)
+            {
+                if (callback.first == family_ || (dispatch_any &&
+                    callback.first == ANY_OK))
+                {
+                    dispatch.push_back(callback.second);
+                } else {
+                    keep.push_back(callback);
+                }
+            }
+            // Put there only the ones that we do not want, drop the rest
+            keep.swap(entry_->callbacks_);
+            keep.clear();
+
+            // We can't keep the lock while we execute callbacks
+            lock.unlock();
+            // Run all the callbacks
             /*
-             * FIXME This approach is not completely exception safe.
-             * If we get an exception from callback, we lose the other
-             * callbacks.
+             * FIXME: This is not completely exception safe. If there's an
+             * exception in a callback, we lose the rest of them.
              */
-            for (map<shared_ptr<ZoneEntry>, NameserverEntry::Callback*>::
-                iterator i(callbacks.begin()); i != callbacks.end(); ++ i)
+            BOOST_FOREACH(const shared_ptr<NameserverEntry::Callback>&
+                callback, dispatch)
             {
-                shared_ptr<ZoneEntry> zone(i->first);
-                (*i->second)(zone);
+                (*callback)(entry_);
+            }
+        }
+
+        // Handle a failure to optain data. Dispatches callbacks and leaves
+        // lock unlocked
+        void failureInternal(Lock &lock) {
+            // Set state of the addresses
+            entry_->expect_address_[family_] = false;
+            entry_->expect_address_[ANY_OK] =
+                entry_->expect_address_[V4_ONLY] ||
+                entry_->expect_address_[V6_ONLY];
+            // When we do not expect any more addresses, decide the state
+            if (!entry_->expect_address_[ANY_OK]) {
+                if (entry_->has_address_[ANY_OK]) {
+                    // We have at last one kind of address, so OK
+                    entry_->setState(READY);
+                } else {
+                    // No addresses :-(
+                    entry_->setState(UNREACHABLE);
+                }
             }
+            // Dispatch any relevant callbacks
+            dispatchCallbacks(lock);
         }
 };
 
-void NameserverEntry::askIP(ResolverInterface& resolver,
-    shared_ptr<ZoneEntry> zone, NameserverEntry::Callback& callback,
+void
+NameserverEntry::askIP(ResolverInterface& resolver, const RRType& type,
+    AddressFamily family, shared_ptr<NameserverEntry> self)
+{
+    QuestionPtr question(new Question(Name(getName()), RRClass(getClass()),
+         type));
+    shared_ptr<ResolverCallback> callback(new ResolverCallback(self, family,
+         type));
+    resolver.resolve(question, callback);
+}
+
+void
+NameserverEntry::askIP(ResolverInterface& resolver,
+    shared_ptr<Callback> callback, AddressFamily family,
     shared_ptr<NameserverEntry> self)
 {
-    if (getState() != Fetchable::NOT_ASKED) {
-        isc_throw(BadValue,
-            "Asking to resolve an IP address, but it was asked before");
+    Lock lock(mutex_);
+
+    if (getState() == EXPIRED || getState() == NOT_ASKED) {
+        // We will request the addresses
+
+        // Set internal state first
+        address_.clear();
+        setState(IN_PROGRESS);
+        has_address_[V4_ONLY] = has_address_[V6_ONLY] = has_address_[ANY_OK] =
+            false;
+        expect_address_[V4_ONLY] = expect_address_[V6_ONLY] =
+            expect_address_[ANY_OK] = true;
+
+        // Store the callback
+        callbacks_.push_back(CallbackPair(family, callback));
+
+        // Ask for both types of addresses
+        askIP(resolver, RRType::A(), V4_ONLY, self);
+        askIP(resolver, RRType::AAAA(), V6_ONLY, self);
+    } else {
+        // We already asked. Do we expect this address type still to come?
+        if (!expect_address_[family]) {
+            // We do not expect it to come, dispatch right away
+            lock.unlock();
+            (*callback)(self);
+            return;
+        } else {
+            // It will come in future, store the callback until then
+            callbacks_.push_back(CallbackPair(family, callback));
+        }
     }
-    setState(Fetchable::IN_PROGRESS);
-    ipCallbacks_[zone] = &callback;
-
-    shared_ptr<ResolverCallback> resolver_callback(new ResolverCallback(self));
-    waiting_responses_ = 2;
-    // TODO Should we ask for both A and AAAA in all occations?
-    resolver.resolve(QuestionPtr(new Question(Name(getName()),
-        RRClass(getClass()), RRType::A())), resolver_callback);
-    resolver.resolve(QuestionPtr(new Question(Name(getName()),
-        RRClass(getClass()), RRType::AAAA())), resolver_callback);
 }
 
 } // namespace dns

src/lib/nsas/nameserver_entry.h

@@ -23,6 +23,7 @@
 
 #include <exceptions/exceptions.h>
 #include <dns/rrset.h>
+#include <dns/rrtype.h>
 
 #include "address_entry.h"
 #include "asiolink.h"
@@ -31,6 +32,7 @@
 #include "hash_key.h"
 #include "lru_list.h"
 #include "fetchable.h"
+#include "resolver_interface.h"
 
 namespace isc {
 namespace nsas {
@@ -81,7 +83,6 @@ class ResolverInterface;
 ///
 /// As this object will be stored in the nameserver address store LRU list,
 /// it is derived from the LRU list entry class.
-/// \todo Is it needed to have virtual functions? Will we subclass it?
 
 class NameserverEntry : public NsasEntry<NameserverEntry>, public Fetchable {
 public:
@@ -112,10 +113,6 @@ public:
     NameserverEntry(const isc::dns::AbstractRRset* v4Set,
         const isc::dns::AbstractRRset* v6Set, time_t curtime = 0);
 
-    /// \brief Virtual Destructor
-    virtual ~NameserverEntry()
-    {}
-
     /// \brief Return Address
     ///
     /// Returns a vector of addresses corresponding to this nameserver.
@@ -135,15 +132,16 @@ public:
     ///     the EXPIRED state by itself. It may be IN_PROGRESS and still
     ///     return some addresses (when one address family arrived and is
     ///     is returned, but the other is still on the way).
-    virtual Fetchable::State getAddresses(
+    /// \todo Should we sort out unreachable addresses as well?
+    Fetchable::State getAddresses(
         NameserverEntry::AddressVector& addresses,
-        AddressRequest family = ANY_OK, bool expired_ok = false);
+        AddressFamily family = ANY_OK, bool expired_ok = false);
 
     // TODO Is this one of any use at all?
     /// \brief Return Address that corresponding to the index
     ///
     /// \param index The address index in the address vector
-    virtual asiolink::IOAddress getAddressAtIndex(uint32_t index) const;
+    asiolink::IOAddress getAddressAtIndex(uint32_t index) const;
 
     /// \brief Update RTT
     ///
@@ -151,28 +149,28 @@ public:
     ///
     /// \param address Address to update
     /// \param RTT New RTT for the address
-    virtual void setAddressRTT(const asiolink::IOAddress& address, uint32_t rtt);
+    void setAddressRTT(const asiolink::IOAddress& address, uint32_t rtt);
 
     /// \brief Update RTT of the address that corresponding to the index
     ///
     /// \param rtt Round-Trip Time
     /// \param index The address's index in address vector
-    virtual void updateAddressRTTAtIndex(uint32_t rtt, uint32_t index);
+    void updateAddressRTTAtIndex(uint32_t rtt, uint32_t index);
 
     /// \brief Set Address Unreachable
     ///
     /// Sets the specified address to be unreachable
     ///
     /// \param address Address to update
-    virtual void setAddressUnreachable(const asiolink::IOAddress& address);
+    void setAddressUnreachable(const asiolink::IOAddress& address);
 
     /// \return Owner Name of RRset
-    virtual std::string getName() const {
+    std::string getName() const {
         return name_;
     }
 
     /// \return Class of RRset
-    virtual short getClass() const {
+    short getClass() const {
         return classCode_;
     }
 
@@ -188,31 +186,15 @@ public:
     /// Returns the expiration time of addresses for this nameserver.  For
     /// simplicity, this quantity is calculated as the minimum expiration time
     /// of the A and AAAA address records.
-    virtual time_t getExpiration() const {
+    time_t getExpiration() const {
         return expiration_;
     }
 
-    /// \brief Predicate for Address Selection
-    ///
-    /// Returns false if the address family of a given entry matches the address
-    /// family given or if the address family is 0 (which means return all
-    /// addresses).  This curious logic is needed for use in the remove_copy_if
-    /// algorithm, which copies all values apart from those for which the
-    /// criteria is met.
-    class AddressSelection : public std::binary_function<short, AddressEntry, bool> {
-    public:
-        bool operator()(short family, const AddressEntry& entry) const {
-            bool match = (entry.getAddress().getFamily() == family) ||
-                (family == 0);
-            return (! match);
-        }
-    };
-
     /// \name Obtaining the IP addresses from resolver
     //@{
     /// \short A callback that some information here arrived (or are unavailable).
     struct Callback {
-        virtual void operator()(NameserverEntry* self) = 0;
+        virtual void operator()(boost::shared_ptr<NameserverEntry> self) = 0;
         /// \short Virtual destructor, so descendants are properly cleaned up
         virtual ~ Callback() {}
     };
@@ -238,25 +220,36 @@ public:
      * \param self Since we need to pass a shared pointer to the resolver, we
      *     need to get one. However, we can not create one from this, because
      *     it would have different reference count. So the caller must pass it.
+     * \return The state the entry is currently in. It can return UNREACHABLE
+     *     even when there are addresses, if there are no addresses for this
+     *     family.
      */
     void askIP(ResolverInterface& resolver,
-        boost::shared_ptr<Callback> callback, AddressRequest family,
+        boost::shared_ptr<Callback> callback, AddressFamily family,
         boost::shared_ptr<NameserverEntry> self);
     //@}
 
 private:
     // TODO Read-write lock?
-    boost::mutex    mutex_;             ///< Mutex protecting this object
+    mutable boost::mutex    mutex_;     ///< Mutex protecting this object
     std::string     name_;              ///< Canonical name of the nameserver
     uint16_t        classCode_;         ///< Class of the nameserver
     std::vector<AddressEntry> address_; ///< Set of V4/V6 addresses
     time_t          expiration_;        ///< Summary expiration time
     time_t          last_access_;       ///< Last access time to the structure
-    // This is our callback class to resolver
+    // Do we have some addresses already? Do we expect some to come?
+    // These are set after asking for IP, if NOT_ASKED, they are uninitialized
+    bool has_address_[ADDR_REQ_MAX], expect_address_[ADDR_REQ_MAX];
+    // Callbacks from resolver
     class ResolverCallback;
     friend class ResolverCallback;
-    // How many responses from resolver do we expect?
-    size_t waiting_responses_;
+    // Callbacks inserted into this object
+    typedef std::pair<AddressFamily, boost::shared_ptr<Callback> >
+        CallbackPair;
+    std::vector<CallbackPair> callbacks_;
+    /// \short Private version that does the actual asking of one address type
+    void askIP(ResolverInterface&, const isc::dns::RRType&, AddressFamily,
+        boost::shared_ptr<NameserverEntry>);
 };
 
 }   // namespace dns

src/lib/nsas/nsas_types.h

@@ -26,13 +26,16 @@ namespace isc {
 namespace nsas {
 
 /// \brief Address requested
-enum AddressRequest {
+enum AddressFamily {
     /// \short Interested only in IPv4 address
     V4_ONLY,
     /// \short Interested only in IPv6 address
     V6_ONLY,
     /// \short Any address is good
-    ANY_OK
+    ANY_OK,
+    /// \short Bumper value, does not mean anything, it just represents the
+    /// max value
+    ADDR_REQ_MAX
 };
 
 }

src/lib/nsas/tests/nameserver_entry_unittest.cc

@@ -111,7 +111,7 @@ protected:
     /// \short Just a really stupid callback counting times called
     struct Callback : public NameserverEntry::Callback {
         size_t count;
-        virtual void operator()(NameserverEntry*) {
+        virtual void operator()(shared_ptr<NameserverEntry>) {
             count ++;
         }
         Callback() : count(0) { }
@@ -213,34 +213,34 @@ TEST_F(NameserverEntryTest, DefaultConstructor) {
 TEST_F(NameserverEntryTest, AddressListConstructor) {
 
     // Initialize with no addresses and check that data returned has size of
-    // zero.
+    // zero and knows it did not ask for the address yet
     NameserverEntry alpha(NULL, NULL);
     NameserverEntry::AddressVector av;
-    alpha.getAddresses(av);
+    EXPECT_EQ(Fetchable::NOT_ASKED, alpha.getAddresses(av));
     EXPECT_EQ(0, av.size());
 
     NameserverEntry::AddressVector av4;
-    alpha.getAddresses(av4, V4_ONLY);
+    EXPECT_EQ(Fetchable::NOT_ASKED, alpha.getAddresses(av4, V4_ONLY));
     EXPECT_EQ(0, av4.size());
 
     NameserverEntry::AddressVector av6;
-    alpha.getAddresses(av6, V6_ONLY);
+    EXPECT_EQ(Fetchable::NOT_ASKED, alpha.getAddresses(av6, V6_ONLY));
     EXPECT_EQ(0, av6.size());
 
     // Initialize with V4 addresses only.
-    EXPECT_TRUE(rrv4_.getRdataCount() > 0);
+    EXPECT_GT(rrv4_.getRdataCount(), 0);
     NameserverEntry beta(&rrv4_, NULL);
 
     NameserverEntry::AddressVector bv;
-    beta.getAddresses(bv);
+    EXPECT_EQ(Fetchable::READY, beta.getAddresses(bv));
     EXPECT_EQ(rrv4_.getRdataCount(), bv.size());
 
     NameserverEntry::AddressVector bv4;
-    beta.getAddresses(bv4, V4_ONLY);
+    EXPECT_EQ(Fetchable::READY, beta.getAddresses(bv4, V4_ONLY));
     EXPECT_EQ(rrv4_.getRdataCount(), bv4.size());
 
     NameserverEntry::AddressVector bv6;
-    beta.getAddresses(bv6, V6_ONLY);
+    EXPECT_EQ(Fetchable::UNREACHABLE, beta.getAddresses(bv6, V6_ONLY));
     EXPECT_EQ(0, bv6.size());
 
     // Check that the addresses received are unique.
@@ -252,15 +252,15 @@ TEST_F(NameserverEntryTest, AddressListConstructor) {
     NameserverEntry gamma(NULL, &rrv6_);
 
     NameserverEntry::AddressVector cv;
-    gamma.getAddresses(cv);
+    EXPECT_EQ(Fetchable::READY, gamma.getAddresses(cv));
     EXPECT_EQ(rrv6_.getRdataCount(), cv.size());
 
     NameserverEntry::AddressVector cv4;
-    gamma.getAddresses(cv4, V4_ONLY);
+    EXPECT_EQ(Fetchable::UNREACHABLE, gamma.getAddresses(cv4, V4_ONLY));
     EXPECT_EQ(0, cv4.size());
 
     NameserverEntry::AddressVector cv6;
-    gamma.getAddresses(cv6, V6_ONLY);
+    EXPECT_EQ(Fetchable::READY, gamma.getAddresses(cv6, V6_ONLY));
     EXPECT_EQ(rrv6_.getRdataCount(), cv6.size());
 
     SCOPED_TRACE("Checking V6 addresses");
@@ -270,17 +270,17 @@ TEST_F(NameserverEntryTest, AddressListConstructor) {
     NameserverEntry delta(&rrv4_, &rrv6_);
 
     NameserverEntry::AddressVector dv;
-    delta.getAddresses(dv);
+    EXPECT_EQ(Fetchable::READY, delta.getAddresses(dv));
     EXPECT_EQ((rrv4_.getRdataCount() + rrv6_.getRdataCount()), dv.size());
 
     NameserverEntry::AddressVector dv4;
-    delta.getAddresses(dv4, V4_ONLY);
+    EXPECT_EQ(Fetchable::READY, delta.getAddresses(dv4, V4_ONLY));
     EXPECT_EQ(rrv4_.getRdataCount(), dv4.size());
     SCOPED_TRACE("Checking V4 addresses after dual-address family constructor");
     CompareAddresses(dv4, rrv4_);
 
     NameserverEntry::AddressVector dv6;
-    delta.getAddresses(dv6, V6_ONLY);
+    EXPECT_EQ(Fetchable::READY, delta.getAddresses(dv6, V6_ONLY));
     EXPECT_EQ(rrv6_.getRdataCount(), dv6.size());
     SCOPED_TRACE("Checking V6 addresses after dual-address family constructor");
     CompareAddresses(dv6, rrv6_);
@@ -288,8 +288,8 @@ TEST_F(NameserverEntryTest, AddressListConstructor) {
     // ... and check that the composite of the v4 and v6 addresses is the same
     // as that returned by the get without a filter.
     NameserverEntry::AddressVector dvcomponent;
-    delta.getAddresses(dvcomponent, V4_ONLY);
-    delta.getAddresses(dvcomponent, V6_ONLY);
+    EXPECT_EQ(Fetchable::READY, delta.getAddresses(dvcomponent, V4_ONLY));
+    EXPECT_EQ(Fetchable::READY, delta.getAddresses(dvcomponent, V6_ONLY));
     SCOPED_TRACE("Checking V4+V6 addresses same as composite return");
     CompareAddressVectors(dv, dvcomponent);
 }
@@ -330,7 +330,7 @@ TEST_F(NameserverEntryTest, SetRTT) {
     NameserverEntry::AddressVector vec;
     alpha.getAddresses(vec);
 
-    EXPECT_TRUE(vec.size() > 0);
+    ASSERT_TRUE(vec.size() > 0);
 
     // Take the first address and change the RTT.
     IOAddress first_address = vec[0].getAddress();
@@ -363,7 +363,7 @@ TEST_F(NameserverEntryTest, Unreachable) {
     NameserverEntry::AddressVector vec;
     alpha.getAddresses(vec);
 
-    EXPECT_TRUE(vec.size() > 0);
+    ASSERT_TRUE(vec.size() > 0);
 
     // Take the first address and mark as unreachable.
     IOAddress first_address = vec[0].getAddress();
@@ -489,7 +489,8 @@ TEST_F(NameserverEntryTest, IPCallbacks) {
     entry->askIP(resolver, callback, V6_ONLY, entry);
 
     // Answer one and see that the callbacks are called
-    resolver.answer(0, Name(EXAMPLE_CO_UK), rdata::in::A("192.0.2.1"));
+    resolver.answer(0, Name(EXAMPLE_CO_UK), RRType::A(),
+        rdata::in::A("192.0.2.1"));
 
     // Both callbacks that want IPv4 should be called by now
     EXPECT_EQ(2, callback->count);
@@ -500,7 +501,8 @@ TEST_F(NameserverEntryTest, IPCallbacks) {
     EXPECT_EQ(1, addresses.size());
     // Answer IPv6 address
     // It is with zero TTL, so it should expire right away
-    resolver.answer(1, Name(EXAMPLE_CO_UK), rdata::in::AAAA("1001:db8::1"), 0);
+    resolver.answer(1, Name(EXAMPLE_CO_UK), RRType::AAAA(),
+        rdata::in::AAAA("2001:db8::1"), 0);
     // The other callback should appear
     EXPECT_EQ(3, callback->count);
     // It should return the one address. It should be expired, but

src/lib/nsas/tests/nsas_test.h

@@ -263,10 +263,11 @@ class TestResolver : public isc::nsas::ResolverInterface {
          * Sends a simple answer to a query.
          * Provide index of a query and the address to pass.
          */
-        void answer(size_t index, const Name& name, const rdata::Rdata& rdata,
-            size_t TTL = 100) {
+        void answer(size_t index, const Name& name, const RRType& type,
+            const rdata::Rdata& rdata, size_t TTL = 100)
+        {
             RRsetPtr set(new RRset(name, RRClass::IN(),
-                RRType::A(), RRTTL(TTL)));
+                type, RRTTL(TTL)));
             set->addRdata(rdata);
             Message address(Message::RENDER); // Not able to create different one
             address.addRRset(Section::ANSWER(), set);