[2210] assorted cleanups

A few more checks in test
cleaned up added headers
reordered log message file

src/bin/auth/auth_messages.mes

@@ -75,6 +75,29 @@ exception type is even more unexpected.  This may rather indicate some
 run time failure than program errors, but in any case the server needs
 to be restarted by hand.
 
+% AUTH_DATASRC_CLIENTS_BUILDER_RECONFIGURE_CONFIG_ERROR Error in configuration: %1
+The thread for maintaining data source clients has received a command to
+reconfigure, but the parameter data is not in the expected format, or contains
+another error. This data should have been validated, but may still contain
+an error. The system is still running with the data sources from before
+the reconfigure command, and the configuration data needs to be checked.
+The specific problem is printed in the log message.
+
+% AUTH_DATASRC_CLIENTS_BUILDER_RECONFIGURE_ERROR Error in configuration: %1
+The thread for maintaining data source clients has received a command to
+reconfigure, but raised an exception while reconfiguring. This is most
+likely a bug in the data source, but it is probably a good idea to verify
+the configuration itself. The system is still running with the data sources
+from before the reconfigure command, and the configuration data needs to be
+checked. The specific problem is printed in the log message.
+
+% AUTH_DATASRC_CLIENTS_BUILDER_RECONFIGURE_UNKNOWN_ERROR Unknown uncaught exception in reconfigure
+The thread for maintaining data source clients has received a command to
+reconfigure, but raised an unknown exception while reconfiguring. This is a
+bug in the data source. The system is still running with the data sources
+from before the reconfigure command, and the configuration data needs to be
+checked.
+
 % AUTH_DATASRC_CLIENTS_BUILDER_STARTED data source builder thread started
 A separate thread for maintaining data source clients has been started.
 

src/bin/auth/datasrc_clients_mgr.h

@@ -61,10 +61,6 @@ enum CommandID {
 typedef std::pair<CommandID, data::ConstElementPtr> Command;
 } // namespace datasrc_clientmgr_internal
 
-typedef std::map<isc::dns::RRClass,
-                 boost::shared_ptr<isc::datasrc::ConfigurableClientList> >
-                    DataSrcClientListMap;
-
 /// \brief Frontend to the manager object for data source clients.
 ///
 /// This class provides interfaces for configuring and updating a set of
@@ -101,7 +97,8 @@ public:
     /// \throw std::bad_alloc internal memory allocation failure.
     /// \throw isc::Unexpected general unexpected system errors.
     DataSrcClientsMgrBase() :
-        builder_(&command_queue_, &cond_, &queue_mutex_, &clients_map_, &map_mutex_),
+        builder_(&command_queue_, &cond_, &queue_mutex_, &clients_map_,
+                 &map_mutex_),
         builder_thread_(boost::bind(&BuilderType::run, &builder_))
     {}
 
@@ -239,18 +236,20 @@ private:
                     configureDataSource(config);
                 typename MutexType::Locker locker(*map_mutex_);
                 std::swap(new_clients_map, *clients_map_);
-            } catch (isc::data::TypeError) {
-                // TODO: log
-                std::cout << "[XX] type error" << std::endl;
-            } catch (std::exception stde) {
-                // TODO: log
-                std::cout << "[XX] bug error" << std::endl;
+                // lock is released by leaving scope
+            } catch (isc::data::TypeError type_error) {
+                LOG_ERROR(auth_logger,
+                    AUTH_DATASRC_CLIENTS_BUILDER_RECONFIGURE_CONFIG_ERROR).
+                    arg(type_error.what());
+            } catch (std::exception exc) {
+                LOG_ERROR(auth_logger,
+                    AUTH_DATASRC_CLIENTS_BUILDER_RECONFIGURE_ERROR).
+                    arg(exc.what());
             } catch (...) {
-                // TODO: log
-                std::cout << "[XX] unknown error" << std::endl;
+                LOG_ERROR(auth_logger,
+                    AUTH_DATASRC_CLIENTS_BUILDER_RECONFIGURE_UNKNOWN_ERROR);
             }
         }
-        // lock
     }
 
     // The following are shared with the manager

src/bin/auth/tests/command_unittest.cc

@@ -18,7 +18,6 @@
 
 #include <util/threads/sync.h>
 
-#include <auth/auth_config.h>
 #include <auth/command.h>
 #include <auth/datasrc_config.h>
 

src/bin/auth/tests/datasrc_clients_builder_unittest.cc

@@ -120,6 +120,7 @@ TEST_F(DataSrcClientsBuilderTest, reconfigure) {
     reconfig_cmd.second = good_config;
     EXPECT_TRUE(builder.handleCommand(reconfig_cmd));
     EXPECT_EQ(1, clients_map->size());
+    EXPECT_EQ(1, map_mutex.lock_count);
 
     // Store the nonempty clients map we now have
     DataSrcClientListsPtr working_config_clients(clients_map);
@@ -130,12 +131,15 @@ TEST_F(DataSrcClientsBuilderTest, reconfigure) {
     reconfig_cmd.second = isc::data::Element::create("{ \"foo\": \"bar\" }");
     EXPECT_TRUE(builder.handleCommand(reconfig_cmd));
     EXPECT_EQ(working_config_clients, clients_map);
+    // Building failed, so map mutex should not have been locked again
+    EXPECT_EQ(1, map_mutex.lock_count);
 
     // The same goes for an empty parameter (it should at least be
     // an empty map)
     reconfig_cmd.second = ConstElementPtr();
     EXPECT_TRUE(builder.handleCommand(reconfig_cmd));
     EXPECT_EQ(working_config_clients, clients_map);
+    EXPECT_EQ(1, map_mutex.lock_count);
 
     // Reconfigure again with the same good clients, the result should
     // be a different map than the original, but not an empty one.
@@ -143,11 +147,16 @@ TEST_F(DataSrcClientsBuilderTest, reconfigure) {
     EXPECT_TRUE(builder.handleCommand(reconfig_cmd));
     EXPECT_NE(working_config_clients, clients_map);
     EXPECT_EQ(1, clients_map->size());
+    EXPECT_EQ(2, map_mutex.lock_count);
 
     // And finally, try an empty config to disable all datasource clients
     reconfig_cmd.second = isc::data::Element::createMap();
     EXPECT_TRUE(builder.handleCommand(reconfig_cmd));
     EXPECT_EQ(0, clients_map->size());
+    EXPECT_EQ(3, map_mutex.lock_count);
+
+    // Also check if it has been cleanly unlocked every time
+    EXPECT_EQ(3, map_mutex.unlock_count);
 }
 
 TEST_F(DataSrcClientsBuilderTest, shutdown) {