Browse Source

[2205] catch exceptions inside builder and log it.

JINMEI Tatuya 12 years ago
parent
commit
15724e5de1

+ 4 - 0
src/bin/auth/auth_messages.mes

@@ -276,3 +276,7 @@ NOTIFY request will not be honored.
 % AUTH_DATASRC_CLIENTS_BUILDER_COMMAND data source builder received command, ID: %1
 
 % AUTH_DATASRC_CLIENTS_MANAGER_UNEXPECTED_STOP data source clients manager stopped unexpectedly, leaving the builder thread
+
+% AUTH_DATASRC_CLIENTS_BUILDER_FAILED data source builder thread stopped due to an exception: %1
+
+% AUTH_DATASRC_CLIENTS_BUILDER_FAILED_UNEXPECTED data source builder thread stopped due to an unexpected exception

+ 28 - 17
src/bin/auth/datasrc_clients_mgr.h

@@ -126,26 +126,37 @@ void
 DataSrcClientsBuilderBase<MutexType, CondVarType>::run() {
     LOG_INFO(auth_logger, AUTH_DATASRC_CLIENTS_BUILDER_STARTED);
 
-    bool keep_running = true;
-    while (keep_running) {
-        std::list<Command> current_commands;
-        {
-            // Move all new commands to local queue under the protection of
-            // queue_mutex_.  Note that list::splice() should never throw.
-            typename MutexType::Locker locker(*queue_mutex_);
-            while (command_queue_->empty()) {
-                cond_->wait(*queue_mutex_);
+    try {
+        bool keep_running = true;
+        while (keep_running) {
+            std::list<Command> current_commands;
+            {
+                // Move all new commands to local queue under the protection of
+                // queue_mutex_.  Note that list::splice() should never throw.
+                typename MutexType::Locker locker(*queue_mutex_);
+                while (command_queue_->empty()) {
+                    cond_->wait(*queue_mutex_);
+                }
+                current_commands.splice(current_commands.end(),
+                                        *command_queue_);
+            } // the lock is release here.
+
+            while (keep_running && !current_commands.empty()) {
+                keep_running = handleCommand(current_commands.front());
+                current_commands.pop_front();
             }
-            current_commands.splice(current_commands.end(), *command_queue_);
-        } // the lock is release here.
-
-        while (keep_running && !current_commands.empty()) {
-            keep_running = handleCommand(current_commands.front());
-            current_commands.pop_front();
         }
-    }
 
-    LOG_INFO(auth_logger, AUTH_DATASRC_CLIENTS_BUILDER_STOPPED);
+        LOG_INFO(auth_logger, AUTH_DATASRC_CLIENTS_BUILDER_STOPPED);
+    } catch (const std::exception& ex) {
+        // We explicitly catch exceptions so we can log it as soon as possible.
+        LOG_ERROR(auth_logger, AUTH_DATASRC_CLIENTS_BUILDER_FAILED).
+            arg(ex.what());
+        throw;
+    } catch (...) {
+        LOG_ERROR(auth_logger, AUTH_DATASRC_CLIENTS_BUILDER_FAILED_UNEXPECTED);
+        throw;
+    }
 }
 
 template <typename MutexType, typename CondVarType>

+ 12 - 0
src/bin/auth/tests/datasrc_clients_builder_unittest.cc

@@ -66,6 +66,18 @@ TEST_F(DataSrcClientsBuilderTest, runMultiCommands) {
     EXPECT_EQ(2, queue_mutex.noop_count);
 }
 
+TEST_F(DataSrcClientsBuilderTest, exception) {
+    // Let the noop command handler throw exceptions and see if we can see
+    // them.
+    command_queue.push_back(noop_cmd);
+    queue_mutex.throw_from_noop = TestMutex::EXCLASS;
+    EXPECT_THROW(builder.run(), isc::Exception);
+
+    command_queue.push_back(noop_cmd);
+    queue_mutex.throw_from_noop = TestMutex::INTEGER;
+    EXPECT_THROW(builder.run(), int);
+}
+
 TEST_F(DataSrcClientsBuilderTest, condWait) {
     // command_queue is originally empty, so it will require waiting on
     // condvar.  specialized wait() will make the delayed command available.

+ 6 - 0
src/bin/auth/tests/datasrc_clients_mgr_unittest.cc

@@ -67,6 +67,12 @@ TEST(DataSrcClientsMgrTest, shutdown) {
     EXPECT_TRUE(FakeDataSrcClientsBuilder::thread_waited);
 }
 
+TEST(DataSrcClientsMgrTest, shutdownWithException) {
+    TestDataSrcClientsMgr mgr;
+    FakeDataSrcClientsBuilder::thread_throw_on_wait = true;
+    EXPECT_THROW(mgr.shutdown(), isc::Unexpected);
+}
+
 TEST(DataSrcClientsMgrTest, realThread) {
     // Using the non-test definition with a real thread.  Just checking
     // no disruption happens.

+ 11 - 0
src/bin/auth/tests/test_datasrc_clients_mgr.cc

@@ -12,6 +12,8 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <exceptions/exceptions.h>
+
 #include "test_datasrc_clients_mgr.h"
 
 namespace isc {
@@ -22,12 +24,21 @@ std::list<internal::Command>* FakeDataSrcClientsBuilder::command_queue = NULL;
 internal::TestCondVar* FakeDataSrcClientsBuilder::cond = NULL;
 internal::TestMutex* FakeDataSrcClientsBuilder::queue_mutex = NULL;
 bool FakeDataSrcClientsBuilder::thread_waited = false;
+bool FakeDataSrcClientsBuilder::thread_throw_on_wait = false;
 
 namespace internal {
 template<>
 void
 TestDataSrcClientsBuilder::doNoop() {
     ++queue_mutex_->noop_count;
+    switch (queue_mutex_->throw_from_noop) {
+    case TestMutex::NONE:
+        break;                  // no throw
+    case TestMutex::EXCLASS:
+        isc_throw(Exception, "test exception");
+    case TestMutex::INTEGER:
+        throw 42;
+    }
 }
 }
 }

+ 29 - 1
src/bin/auth/tests/test_datasrc_clients_mgr.h

@@ -15,6 +15,8 @@
 #ifndef TEST_DATASRC_CLIENTS_MGR_H
 #define TEST_DATASRC_CLIENTS_MGR_H 1
 
+#include <exceptions/exceptions.h>
+
 #include <auth/datasrc_clients_mgr.h>
 
 #include <boost/function.hpp>
@@ -28,11 +30,23 @@ namespace auth {
 namespace internal {
 class TestMutex {
 public:
-    TestMutex() : lock_count(0), unlock_count(0), noop_count(0) {}
+    // for throw_from_noop.
+    // None: no throw from specialized doNoop()
+    // EXCLASS: throw some exception class object
+    // INTEGER: throw an integer
+    enum ExceptionFromNoop { NONE, EXCLASS, INTEGER };
+
+    TestMutex() : lock_count(0), unlock_count(0), noop_count(0),
+                  throw_from_noop(NONE)
+    {}
     class Locker {
     public:
         Locker(TestMutex& mutex) : mutex_(mutex) {
             ++mutex.lock_count;
+            if (mutex.lock_count > 100) { // 100 is an arbitrary choice
+                isc_throw(Unexpected,
+                          "too many test mutex count, likely a bug in test");
+            }
         }
         ~Locker() {
             ++mutex_.unlock_count;
@@ -43,6 +57,7 @@ public:
     size_t lock_count;
     size_t unlock_count;
     size_t noop_count;          // allow doNoop() to modify this
+    ExceptionFromNoop throw_from_noop; // test can set this to control doNoop
 };
 
 class TestCondVar {
@@ -63,6 +78,11 @@ public:
         ++wait_count;
         ++mutex.lock_count;
 
+        if (wait_count > 100) { // 100 is an arbitrary choice
+            isc_throw(Unexpected,
+                      "too many cond wait count, likely a bug in test");
+        }
+
         // make the delayed commands available
         command_queue_->splice(command_queue_->end(), *delayed_command_queue_);
     }
@@ -103,6 +123,7 @@ public:
         FakeDataSrcClientsBuilder::cond = cond;
         FakeDataSrcClientsBuilder::queue_mutex = queue_mutex;
         FakeDataSrcClientsBuilder::thread_waited = false;
+        FakeDataSrcClientsBuilder::thread_throw_on_wait = false;
     }
     void run() {
         FakeDataSrcClientsBuilder::started = true;
@@ -118,6 +139,10 @@ public:
 
     // true iff the manager waited on the thread running the builder.
     static bool thread_waited;
+
+    // If set to true by a test, TestThread::wait() throws an exception
+    // exception.
+    static bool thread_throw_on_wait;
 };
 
 class TestThread {
@@ -127,6 +152,9 @@ public:
     }
     void wait() {
         FakeDataSrcClientsBuilder::thread_waited = true;
+        if (FakeDataSrcClientsBuilder::thread_throw_on_wait) {
+            isc_throw(Unexpected, "for test");
+        }
     }
 };