From: Razvan Becheriu <razvan@isc.org>
Date: Fri, 6 Mar 2020 20:50:17 +0000 (+0200)
Subject: [#1136] check thread id when performing stop
X-Git-Tag: Kea-1.7.6~25
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7970d4411d2d010aff2540f9ef8f58dc28b63087;p=thirdparty%2Fkea.git

[#1136] check thread id when performing stop
---

diff --git a/src/lib/util/tests/thread_pool_unittest.cc b/src/lib/util/tests/thread_pool_unittest.cc
index 4156e9f52c..dde1c9999e 100644
--- a/src/lib/util/tests/thread_pool_unittest.cc
+++ b/src/lib/util/tests/thread_pool_unittest.cc
@@ -19,6 +19,9 @@ using namespace std;
 
 namespace {
 
+/// @brief define CallBack type
+typedef function<void()> CallBack;
+
 /// @brief Test Fixture for testing isc::dhcp::ThreadPool
 class ThreadPoolTest : public ::testing::Test {
 public:
@@ -26,6 +29,12 @@ public:
     ThreadPoolTest() : thread_count_(0), count_(0), wait_(false) {
     }
 
+    /// @brief task function
+    void runTryStopAndWait(ThreadPool<CallBack> &thread_pool) {
+        EXPECT_THROW(thread_pool.stop(), InvalidOperation);
+        EXPECT_NO_THROW(runAndWait());
+    }
+
     /// @brief task function which registers the thread id and signals main
     /// thread to stop waiting and then waits for main thread to signal to exit
     void runAndWait() {
@@ -192,9 +201,6 @@ private:
     list<boost::shared_ptr<std::thread>> threads_;
 };
 
-/// @brief define CallBack type
-typedef function<void()> CallBack;
-
 /// @brief test ThreadPool add and count
 TEST_F(ThreadPoolTest, testAddAndCount) {
     uint32_t items_count;
@@ -386,6 +392,48 @@ TEST_F(ThreadPoolTest, testStartAndStop) {
     ASSERT_EQ(thread_pool.count(), 0);
     // the thread count should be 0
     ASSERT_EQ(thread_pool.size(), 0);
+
+    // create tasks which try to stop the thread pool and then block thread pool
+    // threads until signaled by main thread to force all threads of the thread
+    // pool to run exactly one task
+    call_back = std::bind(&ThreadPoolTest::runTryStopAndWait, this, thread_pool);
+
+    // calling start should create the threads and should keep the queued items
+    EXPECT_NO_THROW(thread_pool.start(thread_count));
+    // the item count should be 0
+    ASSERT_EQ(thread_pool.count(), 0);
+    // the thread count should be 0
+    ASSERT_EQ(thread_pool.size(), thread_count);
+
+    // 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)));
+    }
+
+    // wait for all items to be processed
+    waitTasks(thread_count, items_count);
+    // the item count should be 0
+    ASSERT_EQ(thread_pool.count(), 0);
+    // the thread count should match
+    ASSERT_EQ(thread_pool.size(), thread_count);
+    // as each thread pool thread is still waiting on main to unblock, each
+    // thread should have been registered in ids list
+    checkIds(items_count);
+    // all items should have been processed
+    ASSERT_EQ(count(), items_count);
+
+    // check that the number of processed tasks matches the number of items
+    checkRunHistory(items_count);
+
+    // signal thread pool tasks to continue
+    signalThreads();
+
+    // calling stop should clear all threads and should keep queued items
+    EXPECT_NO_THROW(thread_pool.stop());
+    // the item count should be 0
+    ASSERT_EQ(thread_pool.count(), 0);
+    // the thread count should be 0
+    ASSERT_EQ(thread_pool.size(), 0);
 }
 
 }  // namespace
diff --git a/src/lib/util/thread_pool.h b/src/lib/util/thread_pool.h
index c299be14cd..6c527d5810 100644
--- a/src/lib/util/thread_pool.h
+++ b/src/lib/util/thread_pool.h
@@ -110,6 +110,10 @@ private:
 
     /// @brief stop all the threads
     void stopInternal() {
+        auto id = std::this_thread::get_id();
+        if (checkThreadId(id)) {
+            isc_throw(InvalidOperation, "thread pool stop called by owned thread");
+        }
         queue_.disable();
         for (auto thread : threads_) {
             thread->join();
@@ -117,6 +121,18 @@ private:
         threads_.clear();
     }
 
+    /// @brief check specified thread id against own threads
+    ///
+    /// @return true if thread is owned, false otherwise
+    bool checkThreadId(std::thread::id id) {
+        for (auto thread : threads_) {
+            if (id == thread->get_id()) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     /// @brief Defines a generic thread pool queue.
     ///
     /// The main purpose is to safely manage thread pool tasks.