Better locking

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

src/lib/nsas/zone_entry.cc

@@ -55,7 +55,7 @@ struct ZoneEntry::ResolverCallback : public ResolverInterface::Callback {
         entry_(entry)
     { }
     virtual void success(shared_ptr<AbstractRRset> answer) {
-        Lock lock(entry_->mutex_);
+        shared_ptr<Lock> lock(new Lock(entry_->mutex_));
         RdataIteratorPtr iterator(answer->getRdataIterator());
         iterator->first();
         // If there are no data
@@ -106,34 +106,28 @@ struct ZoneEntry::ResolverCallback : public ResolverInterface::Callback {
             // It is unbelievable, but we found no nameservers there
             if (entry_->nameservers_.empty()) {
                 failureInternal(lock, answer->getTTL().getValue());
+                return;
+            } else {
+                entry_->setState(READY);
+                entry_->expiry_ = answer->getTTL().getValue() + time(NULL);
+                entry_->process(CallbackPtr(), ADDR_REQ_MAX, NULL, lock);
+                return;
             }
         }
     }
     virtual void failure() {
-        Lock lock(entry_->mutex_);
+        shared_ptr<Lock> lock(new Lock(entry_->mutex_));
         /*
          * FIXME: That 5 minutes is just made up and wrong.
          * Where is the correct place to get the correct number?
          */
         failureInternal(lock, 300);
     }
-    void failureInternal(Lock& lock, time_t ttl) {
+    void failureInternal(shared_ptr<Lock> lock, time_t ttl) {
         entry_->setState(UNREACHABLE);
         entry_->expiry_ = ttl + time(NULL);
         // Process all three callback lists and tell them KO
-        // We put them into one set and call that one, so they are
-        // taken out atomicaly
-        entry_->callbacks_[ANY_OK].insert(entry_->callbacks_[ANY_OK].end(),
-            entry_->callbacks_[V4_ONLY].begin(),
-            entry_->callbacks_[V4_ONLY].end());
-        entry_->callbacks_[V4_ONLY].clear();
-        entry_->callbacks_[ANY_OK].insert(entry_->callbacks_[ANY_OK].end(),
-            entry_->callbacks_[V6_ONLY].begin(),
-            entry_->callbacks_[V6_ONLY].end());
-        entry_->callbacks_[V6_ONLY].clear();
-        // The process function will lock by its own
-        lock.unlock();
-        entry_->process(CallbackPtr(), ANY_OK, NULL);
+        entry_->process(CallbackPtr(), ADDR_REQ_MAX, NULL, lock);
     }
     shared_ptr<ZoneEntry> entry_;
 };
@@ -176,13 +170,28 @@ ZoneEntry::addCallback(CallbackPtr callback, AddressFamily family,
     }
 }
 
+namespace {
+
+template<class Container>
+void
+move(Container& into, Container& from) {
+    into.insert(into.end(), from.begin(), from.end());
+    from.clear();
+}
+
+}
+
 void
 ZoneEntry::process(CallbackPtr callback, AddressFamily family,
-    NameserverEntry*)
+    NameserverEntry*, shared_ptr<Lock> lock)
 {
-    Lock lock(mutex_);
+    // If we were not provided with a lock, get one
+    if (!lock) {
+        lock.reset(new Lock(mutex_));
+    }
 
     if (callback) {
+        assert(family != ADDR_REQ_MAX);
         callbacks_[family].push_back(callback);
     }
 
@@ -195,10 +204,15 @@ ZoneEntry::process(CallbackPtr callback, AddressFamily family,
         case UNREACHABLE: {
             // We extract all the callbacks
             vector<CallbackPtr> callbacks;
+            if (family == ADDR_REQ_MAX) {
+                move(callbacks_[ANY_OK], callbacks_[V4_ONLY]);
+                move(callbacks_[ANY_OK], callbacks_[V6_ONLY]);
+                family = ANY_OK;
+            }
             callbacks.swap(callbacks_[family]);
             // We want to call them not locked, so we both do not block the
             // lock and allow them to call our functions
-            lock.unlock();
+            lock->unlock();
             BOOST_FOREACH(const CallbackPtr& callback, callbacks) {
                 callback->unreachable();
             }

src/lib/nsas/zone_entry.h

@@ -126,8 +126,14 @@ private:
     // callbacks. If nameserver is given, it is considered new and valid
     // even if its TTL is 0.
     // The family says which one changed or has any update.
+    // If the familly is ADDR_REQ_MAX, then it means process all callbacks.
+    // However, you must not provide callback.
+    // If lock is provided, it is locked mutex_ and will be used. If not,
+    // will use its own.
     void process(boost::shared_ptr<AddressRequestCallback> callback,
-         AddressFamily family, NameserverEntry* nameserver);
+         AddressFamily family, NameserverEntry* nameserver,
+         boost::shared_ptr<boost::mutex::scoped_lock> lock =
+         boost::shared_ptr<boost::mutex::scoped_lock>());
     // Resolver we use
     boost::shared_ptr<ResolverInterface> resolver_;
     // We store the nameserver table and lru, so we can look up when there's