[2204] extracted swapDataSrcClientLists() from configureDataSource().

now configureDataSource() can take time.

src/bin/auth/datasrc_config.cc

@@ -18,7 +18,7 @@
 
 // This is a trivial specialization for the commonly used version.
 // Defined in .cc to avoid accidental creation of multiple copies.
-void
+AuthSrv::DataSrcClientListsPtr
 configureDataSource(AuthSrv& server, const isc::data::ConstElementPtr& config)
 {
     return (configureDataSourceGeneric<AuthSrv,

src/bin/auth/datasrc_config.h

@@ -17,8 +17,6 @@
 
 #include "auth_srv.h"
 
-#include <util/threads/lock.h>
-
 #include <cc/data.h>
 #include <datasrc/client_list.h>
 
@@ -41,8 +39,9 @@
 /// \param config The configuration value to parse. It is in the form
 ///     as an update from the config manager.
 template<class Server, class List>
-void
-configureDataSourceGeneric(Server& server,
+boost::shared_ptr<std::map<isc::dns::RRClass,
+                           boost::shared_ptr<List> > > // = ListMap below
+configureDataSourceGeneric(Server& /*server*/,
                            const isc::data::ConstElementPtr& config)
 {
     typedef boost::shared_ptr<List> ListPtr;
@@ -63,18 +62,12 @@ configureDataSourceGeneric(Server& server,
                                                                 list));
     }
 
-    // Replace the server's lists.  By ignoring the return value we let the
-    // old lists be destroyed.  Lock will be released immediately after the
-    // swap.
-    {
-        isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
-        server.swapDataSrcClientLists(new_lists);
-    }
+    return (new_lists);
 }
 
 /// \brief Concrete version of configureDataSource() for the
 ///     use with authoritative server implementation.
-void
+AuthSrv::DataSrcClientListsPtr
 configureDataSource(AuthSrv& server, const isc::data::ConstElementPtr& config);
 
 #endif  // DATASRC_CONFIG_H

src/bin/auth/main.cc

@@ -18,6 +18,7 @@
 
 #include <util/buffer.h>
 #include <util/io/socketsession.h>
+#include <util/threads/lock.h>
 
 #include <dns/message.h>
 #include <dns/messagerenderer.h>
@@ -93,18 +94,33 @@ datasrcConfigHandler(AuthSrv* server, bool* first_time,
 {
     assert(server != NULL);
     if (config->contains("classes")) {
+        AuthSrv::DataSrcClientListsPtr lists;
+
         if (*first_time) {
             // HACK: The default is not passed to the handler in the first
             // callback. This one will get the default (or, current value).
             // Further updates will work the usual way.
             assert(config_session != NULL);
             *first_time = false;
-            configureDataSource(*auth_server,
-                                config_session->getRemoteConfigValue(
-                                    "data_sources", "classes"));
+            lists = configureDataSource(
+                *auth_server,
+                config_session->getRemoteConfigValue("data_sources",
+                                                     "classes"));
         } else {
-            configureDataSource(*server, config->get("classes"));
+            lists = configureDataSource(*server, config->get("classes"));
+        }
+
+        // Replace the server's lists.  By ignoring the return value we let the
+        // old lists be destroyed.  Lock will be released immediately after the
+        // swap.
+        {
+            isc::util::thread::Mutex::Locker locker(
+                server->getClientListMutex());
+            lists = server->swapDataSrcClientLists(lists);
         }
+        // The previous lists are destroyed here.  Note that it's outside
+        // of the critical section protected by the locker.  So this can
+        // take time if running on a separate thread.
     }
 }
 

src/bin/auth/tests/auth_srv_unittest.cc

@@ -726,13 +726,21 @@ TEST_F(AuthSrvTest, notifyWithSessionMessageError) {
 }
 
 void
+installDataSrcClientLists(AuthSrv& server,
+                          AuthSrv::DataSrcClientListsPtr lists)
+{
+    thread::Mutex::Locker locker(server.getClientListMutex());
+    server.swapDataSrcClientLists(lists);
+}
+
+void
 updateDatabase(AuthSrv& server, const char* params) {
     const ConstElementPtr config(Element::fromJSON("{"
         "\"IN\": [{"
         "    \"type\": \"sqlite3\","
         "    \"params\": " + string(params) +
         "}]}"));
-    configureDataSource(server, config);
+    installDataSrcClientLists(server, configureDataSource(server, config));
 }
 
 void
@@ -749,7 +757,7 @@ updateInMemory(AuthSrv& server, const char* origin, const char* filename) {
         "   \"type\": \"static\","
         "   \"params\": \"" + string(STATIC_DSRC_FILE) + "\""
         "}]}"));
-    configureDataSource(server, config);
+    installDataSrcClientLists(server, configureDataSource(server, config));
 }
 
 void
@@ -759,7 +767,7 @@ updateBuiltin(AuthSrv& server) {
         "   \"type\": \"static\","
         "   \"params\": \"" + string(STATIC_DSRC_FILE) + "\""
         "}]}"));
-    configureDataSource(server, config);
+    installDataSrcClientLists(server, configureDataSource(server, config));
 }
 
 // Try giving the server a TSIG signed request and see it can anwer signed as
@@ -957,7 +965,7 @@ TEST_F(AuthSrvTest, updateWithInMemoryClient) {
         "   \"params\": {},"
         "   \"cache-enable\": true"
         "}]}"));
-    configureDataSource(server, config);
+    installDataSrcClientLists(server, configureDataSource(server, config));
     // after successful configuration, we should have one (with empty zoneset).
 
     // The memory data source is empty, should return REFUSED rcode.

src/bin/auth/tests/command_unittest.cc

@@ -192,6 +192,14 @@ zoneChecks(AuthSrv& server) {
 }
 
 void
+installDataSrcClientLists(AuthSrv& server,
+                          AuthSrv::DataSrcClientListsPtr lists)
+{
+    isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
+    server.swapDataSrcClientLists(lists);
+}
+
+void
 configureZones(AuthSrv& server) {
     ASSERT_EQ(0, system(INSTALL_PROG " -c " TEST_DATA_DIR "/test1.zone.in "
                         TEST_DATA_BUILDDIR "/test1.zone.copied"));
@@ -210,7 +218,7 @@ configureZones(AuthSrv& server) {
         "   \"cache-enable\": true"
         "}]}"));
 
-    configureDataSource(server, config);
+    installDataSrcClientLists(server, configureDataSource(server, config));
 
     zoneChecks(server);
 }
@@ -273,7 +281,7 @@ TEST_F(AuthCommandTest,
         "    \"cache-enable\": true,"
         "    \"cache-zones\": [\"example.org\"]"
         "}]}"));
-    configureDataSource(server_, config);
+    installDataSrcClientLists(server_, configureDataSource(server_, config));
 
     {
         isc::util::thread::Mutex::Locker locker(server_.getClientListMutex());

src/bin/auth/tests/datasrc_config_unittest.cc

@@ -60,14 +60,11 @@ private:
 
 typedef shared_ptr<FakeList> ListPtr;
 
+// Forward declaration.  We need precise definition of DatasrcConfigTest
+// to complete this function.
 void
 testConfigureDataSource(DatasrcConfigTest& test,
-                        const isc::data::ConstElementPtr& config)
-{
-    // We use the test fixture for the Server type.  This makes it possible
-    // to easily fake all needed methods and look that they were called.
-    configureDataSourceGeneric<DatasrcConfigTest, FakeList>(test, config);
-}
+                        const isc::data::ConstElementPtr& config);
 
 void
 datasrcConfigHandler(DatasrcConfigTest* fake_server, const std::string&,
@@ -162,6 +159,17 @@ protected:
     mutable isc::util::thread::Mutex mutex_;
 };
 
+void
+testConfigureDataSource(DatasrcConfigTest& test,
+                        const isc::data::ConstElementPtr& config)
+{
+    // We use the test fixture for the Server type.  This makes it possible
+    // to easily fake all needed methods and look that they were called.
+    shared_ptr<std::map<dns::RRClass, ListPtr> > lists =
+        configureDataSourceGeneric<DatasrcConfigTest, FakeList>(test, config);
+    test.swapDataSrcClientLists(lists);
+}
+
 // Push there a configuration with a single list.
 TEST_F(DatasrcConfigTest, createList) {
     initializeINList();