Use shared_from_this instead of passing self

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

src/lib/nsas/TODO

@@ -22,4 +22,3 @@ Long term:
   solution to use weak_ptr inside the hash_table instead of shared_ptr and
   catch the exception inside get() (and getOrAdd) and delete the dead pointer.
 * Selection algorithm
-* Use enable_shared_from_this.hpp instead of passing self everywhere

src/lib/nsas/nameserver_entry.cc

@@ -304,19 +304,18 @@ class NameserverEntry::ResolverCallback : public ResolverInterface::Callback {
 
 void
 NameserverEntry::askIP(shared_ptr<ResolverInterface> resolver,
-    const RRType& type, AddressFamily family, shared_ptr<NameserverEntry> self)
+    const RRType& type, AddressFamily family)
 {
     QuestionPtr question(new Question(Name(getName()), RRClass(getClass()),
-         type));
-    shared_ptr<ResolverCallback> callback(new ResolverCallback(self, family,
-         type));
+        type));
+    shared_ptr<ResolverCallback> callback(new ResolverCallback(
+        shared_from_this(), family, type));
     resolver->resolve(question, callback);
 }
 
 void
 NameserverEntry::askIP(shared_ptr<ResolverInterface> resolver,
-    shared_ptr<Callback> callback, AddressFamily family,
-    shared_ptr<NameserverEntry> self)
+    shared_ptr<Callback> callback, AddressFamily family)
 {
     Lock lock(mutex_);
 
@@ -339,14 +338,14 @@ NameserverEntry::askIP(shared_ptr<ResolverInterface> resolver,
         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);
+        askIP(resolver, RRType::A(), V4_ONLY);
+        askIP(resolver, RRType::AAAA(), V6_ONLY);
     } 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);
+            (*callback)(shared_from_this());
             return;
         } else {
             // It will come in future, store the callback until then

src/lib/nsas/nameserver_entry.h

@@ -20,6 +20,7 @@
 #include <string>
 #include <vector>
 #include <boost/thread.hpp>
+#include <boost/enable_shared_from_this.hpp>
 
 #include <exceptions/exceptions.h>
 #include <dns/rrset.h>
@@ -83,8 +84,12 @@ class ResolverInterface;
 ///
 /// As this object will be stored in the nameserver address store LRU list,
 /// it is derived from the LRU list entry class.
+///
+/// It uses shared_from_this in its methods. It must live inside a shared_ptr.
 
-class NameserverEntry : public NsasEntry<NameserverEntry>, public Fetchable {
+class NameserverEntry : public NsasEntry<NameserverEntry>, public Fetchable,
+    public boost::enable_shared_from_this<NameserverEntry>
+{
 public:
     /// List of addresses associated with this nameserver
     typedef std::vector<AddressEntry>   AddressVector;
@@ -205,16 +210,12 @@ public:
      *     not change which adresses are requested, but the callback might
      *     be executed when at last one requested type is available (eg. not
      *     waiting for the other one).
-     * \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(boost::shared_ptr<ResolverInterface> resolver,
-        boost::shared_ptr<Callback> callback, AddressFamily family,
-        boost::shared_ptr<NameserverEntry> self);
+        boost::shared_ptr<Callback> callback, AddressFamily family);
     //@}
 
 private:
@@ -236,8 +237,7 @@ private:
     std::vector<CallbackPair> callbacks_;
     /// \short Private version that does the actual asking of one address type
     void askIP(boost::shared_ptr<ResolverInterface> resolver,
-        const isc::dns::RRType&, AddressFamily,
-        boost::shared_ptr<NameserverEntry>);
+        const isc::dns::RRType&, AddressFamily);
 };
 
 }   // namespace dns

src/lib/nsas/tests/nameserver_address_unittest.cc

@@ -48,7 +48,7 @@ public:
 
         ns_.reset(new NameserverEntry(name_.toText(), RRClass::IN()));
         shared_ptr<TestResolver> resolver(new TestResolver);
-        ns_->askIP(resolver, shared_ptr<Callback>(new Callback), ANY_OK, ns_);
+        ns_->askIP(resolver, shared_ptr<Callback>(new Callback), ANY_OK);
         resolver->asksIPs(name_, 0, 1);
         resolver->requests[0].second->success(rrv4_);
     }

src/lib/nsas/tests/nameserver_entry_unittest.cc

@@ -76,7 +76,7 @@ protected:
         shared_ptr<TestResolver> resolver(new TestResolver);
         shared_ptr<Callback> callback(new Callback);
         // Let it ask for data
-        entry->askIP(resolver, callback, ANY_OK, entry);
+        entry->askIP(resolver, callback, ANY_OK);
         // Check it really asked and sort the queries
         resolver->asksIPs(Name(entry->getName()), 0, 1);
         // Respond with answers
@@ -256,7 +256,7 @@ TEST_F(NameserverEntryTest, IPCallbacks) {
     shared_ptr<Callback> callback(new Callback);
     shared_ptr<TestResolver> resolver(new TestResolver);
 
-    entry->askIP(resolver, callback, ANY_OK, entry);
+    entry->askIP(resolver, callback, ANY_OK);
     // Ensure it becomes IN_PROGRESS
     EXPECT_EQ(Fetchable::IN_PROGRESS, entry->getState());
     // Now, there should be two queries in the resolver
@@ -264,12 +264,12 @@ TEST_F(NameserverEntryTest, IPCallbacks) {
     ASSERT_TRUE(resolver->asksIPs(Name(EXAMPLE_CO_UK), 0, 1));
 
     // Another one might ask
-    entry->askIP(resolver, callback, V4_ONLY, entry);
+    entry->askIP(resolver, callback, V4_ONLY);
     // There should still be only two queries in the resolver
     ASSERT_EQ(2, resolver->requests.size());
 
     // Another one, with need of IPv6 address
-    entry->askIP(resolver, callback, V6_ONLY, entry);
+    entry->askIP(resolver, callback, V6_ONLY);
 
     // Answer one and see that the callbacks are called
     resolver->answer(0, Name(EXAMPLE_CO_UK), RRType::A(),
@@ -306,7 +306,7 @@ TEST_F(NameserverEntryTest, IPCallbacksUnreachable) {
     shared_ptr<TestResolver> resolver(new TestResolver);
 
     // Ask for its IP
-    entry->askIP(resolver, callback, ANY_OK, entry);
+    entry->askIP(resolver, callback, ANY_OK);
     // Check it asks the resolver
     EXPECT_EQ(2, resolver->requests.size());
     ASSERT_TRUE(resolver->asksIPs(Name(EXAMPLE_CO_UK), 0, 1));

src/lib/nsas/tests/zone_entry_unittest.cc

@@ -98,7 +98,7 @@ TEST_F(ZoneEntryTest, CallbackNoNS) {
         EXAMPLE_CO_UK, RRClass::IN(), nameserver_table_, nameserver_lru_));
     EXPECT_EQ(Fetchable::NOT_ASKED, zone->getState());
     // It should accept the callback
-    zone->addCallback(callback_, ANY_OK, zone);
+    zone->addCallback(callback_, ANY_OK);
     EXPECT_EQ(Fetchable::IN_PROGRESS, zone->getState());
     // Ask for the nameservers
     EXPECT_NO_THROW(resolver_->provideNS(0, rr_empty_));
@@ -115,7 +115,7 @@ TEST_F(ZoneEntryTest, CallbackZeroTTL) {
         EXAMPLE_CO_UK, RRClass::IN(), nameserver_table_, nameserver_lru_));
     EXPECT_EQ(Fetchable::NOT_ASKED, zone->getState());
     // It should accept the callback
-    zone->addCallback(callback_, ANY_OK, zone);
+    zone->addCallback(callback_, ANY_OK);
     EXPECT_EQ(Fetchable::IN_PROGRESS, zone->getState());
     EXPECT_NO_THROW(resolver_->provideNS(0, rr_single_));
     // It should not be answered yet, it should ask for the IP addresses
@@ -123,7 +123,7 @@ TEST_F(ZoneEntryTest, CallbackZeroTTL) {
     EXPECT_EQ(0, callback_->unreachable_count_);
     resolver_->asksIPs(ns_name_, 1, 2);
     // It should request the NSs again, as TTL is 0
-    zone->addCallback(callback_, ANY_OK, zone);
+    zone->addCallback(callback_, ANY_OK);
     EXPECT_EQ(4, resolver_->requests.size());
 }
 
@@ -134,7 +134,7 @@ TEST_F(ZoneEntryTest, CallbacksAnswered) {
     // It should be in NOT_ASKED state
     EXPECT_EQ(Fetchable::NOT_ASKED, zone->getState());
     // It should accept the callback
-    zone->addCallback(callback_, ANY_OK, zone);
+    zone->addCallback(callback_, ANY_OK);
     EXPECT_EQ(Fetchable::IN_PROGRESS, zone->getState());
     EXPECT_NO_THROW(resolver_->provideNS(0, rr_single_));
     // It should not be answered yet, it should ask for the IP addresses
@@ -145,8 +145,8 @@ TEST_F(ZoneEntryTest, CallbacksAnswered) {
     // (not that they are ready)
     EXPECT_EQ(Fetchable::READY, zone->getState());
     // Give two more callbacks, with different address families
-    zone->addCallback(callback_, V4_ONLY, zone);
-    zone->addCallback(callback_, V6_ONLY, zone);
+    zone->addCallback(callback_, V4_ONLY);
+    zone->addCallback(callback_, V6_ONLY);
     // Nothing more is asked
     EXPECT_EQ(3, resolver_->requests.size());
     EXPECT_NO_THROW(resolver_->answer(1, ns_name_, RRType::A(),
@@ -166,7 +166,7 @@ TEST_F(ZoneEntryTest, CallbacksAnswered) {
     // It should think it is ready
     EXPECT_EQ(Fetchable::READY, zone->getState());
     // When we ask something more, it should be answered right away
-    zone->addCallback(callback_, V4_ONLY, zone);
+    zone->addCallback(callback_, V4_ONLY);
     EXPECT_EQ(3, resolver_->requests.size());
     ASSERT_EQ(4, callback_->successes_.size());
     EXPECT_TRUE(IOAddress("192.0.2.1").equal(callback_->successes_[3]));
@@ -180,7 +180,7 @@ TEST_F(ZoneEntryTest, CallbacksAOnly) {
     // It should be in NOT_ASKED state
     EXPECT_EQ(Fetchable::NOT_ASKED, zone->getState());
     // It should accept the callback
-    zone->addCallback(callback_, ANY_OK, zone);
+    zone->addCallback(callback_, ANY_OK);
     EXPECT_EQ(Fetchable::IN_PROGRESS, zone->getState());
     EXPECT_NO_THROW(resolver_->provideNS(0, rr_single_));
     // It should not be answered yet, it should ask for the IP addresses
@@ -189,8 +189,8 @@ TEST_F(ZoneEntryTest, CallbacksAOnly) {
     resolver_->asksIPs(ns_name_, 1, 2);
     EXPECT_EQ(Fetchable::READY, zone->getState());
     // Give two more callbacks, with different address families
-    zone->addCallback(callback_, V4_ONLY, zone);
-    zone->addCallback(callback_, V6_ONLY, zone);
+    zone->addCallback(callback_, V4_ONLY);
+    zone->addCallback(callback_, V6_ONLY);
     ASSERT_GE(resolver_->requests.size(), 3);
     resolver_->requests[2].second->failure();
     // One should be rejected, but two still stay, they have chance
@@ -206,13 +206,13 @@ TEST_F(ZoneEntryTest, CallbacksAOnly) {
     // Everything arriwed, so we are ready
     EXPECT_EQ(Fetchable::READY, zone->getState());
     // Try asking something more
-    zone->addCallback(callback_, V4_ONLY, zone);
+    zone->addCallback(callback_, V4_ONLY);
     EXPECT_EQ(3, resolver_->requests.size());
     ASSERT_EQ(3, callback_->successes_.size());
     EXPECT_TRUE(IOAddress("192.0.2.1").equal(callback_->successes_[2]));
     EXPECT_EQ(1, callback_->unreachable_count_);
 
-    zone->addCallback(callback_, V6_ONLY, zone);
+    zone->addCallback(callback_, V6_ONLY);
     EXPECT_EQ(3, resolver_->requests.size());
     EXPECT_EQ(3, callback_->successes_.size());
     EXPECT_EQ(2, callback_->unreachable_count_);
@@ -225,7 +225,7 @@ TEST_F(ZoneEntryTest, CallbackTwoNS) {
     // It should be in NOT_ASKED state
     EXPECT_EQ(Fetchable::NOT_ASKED, zone->getState());
     // It should accept the callback
-    zone->addCallback(callback_, V4_ONLY, zone);
+    zone->addCallback(callback_, V4_ONLY);
     EXPECT_EQ(Fetchable::IN_PROGRESS, zone->getState());
     EXPECT_NO_THROW(resolver_->provideNS(0, rrns_));
     EXPECT_EQ(Fetchable::READY, zone->getState());
@@ -246,15 +246,15 @@ TEST_F(ZoneEntryTest, CallbackTwoNS) {
     EXPECT_EQ(0, callback_->successes_.size());
     // And question for v6 or any should still wait while v4 should be failed
     // right away
-    zone->addCallback(callback_, V6_ONLY, zone);
+    zone->addCallback(callback_, V6_ONLY);
     EXPECT_EQ(1, callback_->unreachable_count_);
     EXPECT_EQ(0, callback_->successes_.size());
 
-    zone->addCallback(callback_, ANY_OK, zone);
+    zone->addCallback(callback_, ANY_OK);
     EXPECT_EQ(1, callback_->unreachable_count_);
     EXPECT_EQ(0, callback_->successes_.size());
 
-    zone->addCallback(callback_, V4_ONLY, zone);
+    zone->addCallback(callback_, V4_ONLY);
     EXPECT_EQ(2, callback_->unreachable_count_);
     EXPECT_EQ(0, callback_->successes_.size());
     // Answer the IPv6 one

src/lib/nsas/zone_entry.cc

@@ -126,7 +126,7 @@ class ZoneEntry::ResolverCallback : public ResolverInterface::Callback {
                     entry_->setState(READY);
                     entry_->expiry_ = answer->getTTL().getValue() + time(NULL);
                     entry_->process(CallbackPtr(), ADDR_REQ_MAX,
-                        NameserverPtr(), entry_, lock);
+                        NameserverPtr(), lock);
                     return;
                 }
             }
@@ -145,15 +145,13 @@ class ZoneEntry::ResolverCallback : public ResolverInterface::Callback {
             entry_->expiry_ = ttl + time(NULL);
             // Process all three callback lists and tell them KO
             entry_->process(CallbackPtr(), ADDR_REQ_MAX, NameserverPtr(),
-                entry_, lock);
+                lock);
         }
         shared_ptr<ZoneEntry> entry_;
 };
 
 void
-ZoneEntry::addCallback(CallbackPtr callback, AddressFamily family,
-    shared_ptr<ZoneEntry> self)
-{
+ZoneEntry::addCallback(CallbackPtr callback, AddressFamily family) {
     Lock lock(mutex_);
 
     bool ask(false);
@@ -174,7 +172,7 @@ ZoneEntry::addCallback(CallbackPtr callback, AddressFamily family,
     } else {
         // Try to process it right away, store if not possible to handle
         lock.unlock();
-        process(callback, family, NameserverPtr(), self);
+        process(callback, family, NameserverPtr());
         return;
     }
 
@@ -183,7 +181,7 @@ ZoneEntry::addCallback(CallbackPtr callback, AddressFamily family,
         QuestionPtr question(new Question(Name(name_), class_code_,
             RRType::NS()));
         shared_ptr<ResolverCallback> resolver_callback(
-            new ResolverCallback(self));
+            new ResolverCallback(shared_from_this()));
         resolver_->resolve(question, resolver_callback);
     }
 }
@@ -243,7 +241,7 @@ class ZoneEntry::NameserverCallback : public NameserverEntry::Callback {
             family_(family)
         { }
         virtual void operator()(NameserverPtr ns) {
-            entry_->process(CallbackPtr(), family_, ns, entry_);
+            entry_->process(CallbackPtr(), family_, ns);
         }
     private:
         shared_ptr<ZoneEntry> entry_;
@@ -270,8 +268,7 @@ ZoneEntry::dispatchFailures(AddressFamily family, shared_ptr<Lock> lock) {
 
 void
 ZoneEntry::process(CallbackPtr callback, AddressFamily family,
-    shared_ptr<NameserverEntry> nameserver, shared_ptr<ZoneEntry> self,
-    shared_ptr<Lock> lock)
+    shared_ptr<NameserverEntry> nameserver, shared_ptr<Lock> lock)
 {
     // If we were not provided with a lock, get one
     if (!lock) {
@@ -297,9 +294,9 @@ ZoneEntry::process(CallbackPtr callback, AddressFamily family,
         case READY:
             if (family == ADDR_REQ_MAX) {
                 // Just process each one separately
-                process(CallbackPtr(), ANY_OK, nameserver, self, lock);
-                process(CallbackPtr(), V4_ONLY, nameserver, self, lock);
-                process(CallbackPtr(), V6_ONLY, nameserver, self, lock);
+                process(CallbackPtr(), ANY_OK, nameserver, lock);
+                process(CallbackPtr(), V4_ONLY, nameserver, lock);
+                process(CallbackPtr(), V6_ONLY, nameserver, lock);
             } else {
                 // Nothing to do anyway for this family, be dormant
                 if (callbacks_[family].empty()) {
@@ -351,12 +348,12 @@ ZoneEntry::process(CallbackPtr callback, AddressFamily family,
                     // be called directly from the askIP again
                     lock->unlock();
                     shared_ptr<NameserverCallback> ns_callbacks[ADDR_REQ_MAX];;
-                    ns_callbacks[ANY_OK].reset(new NameserverCallback(self,
-                        ANY_OK));
-                    ns_callbacks[V4_ONLY].reset(new NameserverCallback(self,
-                        V4_ONLY));
-                    ns_callbacks[V6_ONLY].reset(new NameserverCallback(self,
-                        V6_ONLY));
+                    ns_callbacks[ANY_OK].reset(new NameserverCallback(
+                        shared_from_this(), ANY_OK));
+                    ns_callbacks[V4_ONLY].reset(new NameserverCallback(
+                        shared_from_this(), V4_ONLY));
+                    ns_callbacks[V6_ONLY].reset(new NameserverCallback(
+                        shared_from_this(), V6_ONLY));
                     /*
                      * TODO: Possible place for an optimisation. We now ask
                      * everything we can. We should limit this to something like
@@ -370,16 +367,13 @@ ZoneEntry::process(CallbackPtr callback, AddressFamily family,
                         // callback for different one.
                         // If they recurse back to us (call directly), we kill
                         // it by the in_process_
-                        ns->askIP(resolver_, ns_callbacks[V4_ONLY], V4_ONLY,
-                            ns);
-                        ns->askIP(resolver_, ns_callbacks[V6_ONLY], V6_ONLY,
-                            ns);
-                        ns->askIP(resolver_, ns_callbacks[ANY_OK], ANY_OK,
-                            ns);
+                        ns->askIP(resolver_, ns_callbacks[V4_ONLY], V4_ONLY);
+                        ns->askIP(resolver_, ns_callbacks[V6_ONLY], V6_ONLY);
+                        ns->askIP(resolver_, ns_callbacks[ANY_OK], ANY_OK);
                     }
                     // Retry with all the data that might have arrived
                     in_process_[family] = false;
-                    process(callback, family, nameserver, self);
+                    process(callback, family, nameserver);
                     // And be done
                     return;
                 // We have some addresses to answer

src/lib/nsas/zone_entry.h

@@ -21,6 +21,7 @@
 #include <vector>
 #include <boost/thread.hpp>
 #include <boost/shared_ptr.hpp>
+#include <boost/enable_shared_from_this.hpp>
 
 #include <dns/rrset.h>
 
@@ -45,8 +46,12 @@ class AddressRequestCallback;
 /// Although the interface is simple, the internal processing is fairly
 /// complicated, in that the class takes account of triggering fetches for
 /// addresses of nameservers when the address records expire.
+///
+/// It uses shared_from_this in its methods. It must live inside a shared_ptr.
 
-class ZoneEntry : public NsasEntry<ZoneEntry>, public Fetchable {
+class ZoneEntry : public NsasEntry<ZoneEntry>, public Fetchable,
+    public boost::enable_shared_from_this<ZoneEntry>
+{
 public:
 
     /**
@@ -89,14 +94,9 @@ public:
      *
      * \param callback The callback itself.
      * \param family Which address family is acceptable as an answer?
-     * \param self A shared pointer to this zone entry. It is not possible to
-     *     create one from C++ this pointer, since another shared pointer
-     *     will already exist at that point, however it is needed to callback.
-     *     When calling function on the zone entry, you should already have
-     *     one.
      */
     void addCallback(boost::shared_ptr<AddressRequestCallback>
-        callback, AddressFamily family, boost::shared_ptr<ZoneEntry> self);
+        callback, AddressFamily family);
 
     /// \short Protected members, so they can be accessed by tests.
     //@{
@@ -128,7 +128,6 @@ private:
     // will use its own. It might unlock the lock.
     void process(boost::shared_ptr<AddressRequestCallback> callback,
          AddressFamily family, boost::shared_ptr<NameserverEntry> nameserver,
-         boost::shared_ptr<ZoneEntry> self,
          boost::shared_ptr<boost::mutex::scoped_lock> lock =
          boost::shared_ptr<boost::mutex::scoped_lock>());
     // Resolver we use