From: Razvan Becheriu <razvan@isc.org>
Date: Tue, 31 Mar 2020 19:33:29 +0000 (+0300)
Subject: [#1089] refactored and fixed thread pool tests
X-Git-Tag: Kea-1.7.7~93
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=63f8149e4a189da5b8ddb3012a5805bdf3e53e4b;p=thirdparty%2Fkea.git

[#1089] refactored and fixed thread pool tests
---

diff --git a/src/lib/util/tests/thread_pool_unittest.cc b/src/lib/util/tests/thread_pool_unittest.cc
index 5658910421..e10a5871c6 100644
--- a/src/lib/util/tests/thread_pool_unittest.cc
+++ b/src/lib/util/tests/thread_pool_unittest.cc
@@ -60,9 +60,13 @@ public:
     /// @brief task function which tries to stop the thread pool and then calls
     /// @ref runAndWait
     void runStopResetAndWait(ThreadPool<CallBack>* thread_pool) {
-        EXPECT_THROW(thread_pool->stop(), InvalidOperation);
-        EXPECT_THROW(thread_pool->reset(), InvalidOperation);
-        EXPECT_NO_THROW(runAndWait());
+        static std::mutex lock;
+        {
+            std::lock_guard<std::mutex> lk(lock);
+            EXPECT_THROW(thread_pool->stop(), InvalidOperation);
+            EXPECT_THROW(thread_pool->reset(), InvalidOperation);
+        }
+        runAndWait();
     }
 
     /// @brief reset all counters and internal test state
@@ -415,7 +419,7 @@ TEST_F(ThreadPoolTest, testStartAndStop) {
 
     // add items to running thread pool
     for (uint32_t i = 0; i < items_count; ++i) {
-        EXPECT_NO_THROW(thread_pool.add(boost::make_shared<CallBack>(call_back)));
+        thread_pool.add(boost::make_shared<CallBack>(call_back));
     }
 
     // wait for all items to be processed
diff --git a/src/lib/util/watched_thread.cc b/src/lib/util/watched_thread.cc
index df7a43917b..6cd86ab8b9 100644
--- a/src/lib/util/watched_thread.cc
+++ b/src/lib/util/watched_thread.cc
@@ -15,10 +15,7 @@ WatchedThread::start(const boost::function<void()>& thread_main) {
     clearReady(ERROR);
     clearReady(READY);
     clearReady(TERMINATE);
-    {
-	std::lock_guard<std::mutex> lock(last_error_mutex_);
-	last_error_ = "no error";
-    }
+    setErrorInternal("no error");
     thread_.reset(new std::thread(thread_main));
 }
 
@@ -62,23 +59,26 @@ WatchedThread::stop() {
 
     clearReady(ERROR);
     clearReady(READY);
-    std::lock_guard<std::mutex> lock(last_error_mutex_);
-    last_error_ = "thread stopped";
+    setErrorInternal("thread stopped");
+}
+
+void
+WatchedThread::setErrorInternal(const std::string& error_msg) {
+    std::lock_guard<std::mutex> lock(mutex_);
+    last_error_ = error_msg;
 }
 
 void
 WatchedThread::setError(const std::string& error_msg) {
-    {
-        std::lock_guard<std::mutex> lock(last_error_mutex_);
-        last_error_ = error_msg;
-    }
+    setErrorInternal(error_msg);
     markReady(ERROR);
 }
 
 std::string
 WatchedThread::getLastError() {
-    std::lock_guard<std::mutex> lock(last_error_mutex_);
+    std::lock_guard<std::mutex> lock(mutex_);
     return (last_error_);
 }
-} // end of namespace isc::util
-} // end of namespace isc
+
+}  // namespace util
+}  // namespace isc
diff --git a/src/lib/util/watched_thread.h b/src/lib/util/watched_thread.h
index 1cf9b4c34c..d0f9203adb 100644
--- a/src/lib/util/watched_thread.h
+++ b/src/lib/util/watched_thread.h
@@ -99,7 +99,7 @@ public:
     /// This records the given error message and sets the error watch
     /// socket to ready.
     ///
-    /// @param error_msg
+    /// @param error_msg to be set as last error
     void setError(const std::string& error_msg);
 
     /// @brief Fetches the error message text for the most recent error
@@ -107,11 +107,20 @@ public:
     /// @return string containing the error message
     std::string getLastError();
 
+private:
+
+    /// @brief Sets the error state thread safe
+    ///
+    /// This records the given error message
+    ///
+    /// @param error_msg to be set as last error
+    void setErrorInternal(const std::string& error_msg);
+
     /// @brief Error message of the last error encountered
     std::string last_error_;
 
-    /// @brief Mutex to protect last error message concurrent access
-    std::mutex last_error_mutex_;
+    /// @brief Mutex to protect internal state
+    std::mutex mutex_;
 
     /// @brief WatchSockets that are used to communicate with the owning thread
     /// There are three:
@@ -130,7 +139,7 @@ public:
 /// @brief Defines a pointer to a WatchedThread
 typedef boost::shared_ptr<WatchedThread> WatchedThreadPtr;
 
-}; // namespace isc::util
-}; // namespace isc
+}  // namespace util
+}  // namespace isc
 
 #endif // WATCHED_THREAD_H