From: Razvan Becheriu Date: Mon, 17 May 2021 14:34:02 +0000 (+0300) Subject: [#1818] clean up code X-Git-Tag: Kea-1.9.8~79 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=35bc17e59317b31b545f6ca32ade23e8bf4c834f;p=thirdparty%2Fkea.git [#1818] clean up code --- diff --git a/src/lib/config/cmd_http_listener.h b/src/lib/config/cmd_http_listener.h index 1a57a4e401..4b1f414218 100644 --- a/src/lib/config/cmd_http_listener.h +++ b/src/lib/config/cmd_http_listener.h @@ -42,9 +42,13 @@ public: void start(); /// @brief Pauses the listener's thread pool. + /// + /// Suspends thread pool event processing. void pause(); /// @brief Resumes running the listener's thread pool. + /// + /// Resumes thread pool event processing. void resume(); /// @brief Stops the listener's thread pool. diff --git a/src/lib/config/tests/cmd_http_listener_unittests.cc b/src/lib/config/tests/cmd_http_listener_unittests.cc index bddf296997..be700b3b71 100644 --- a/src/lib/config/tests/cmd_http_listener_unittests.cc +++ b/src/lib/config/tests/cmd_http_listener_unittests.cc @@ -52,9 +52,10 @@ public: /// Starts test timer which detects timeouts, deregisters all commands /// from CommandMgr, and enables multi-threading mode. CmdHttpListenerTest() - : listener_(), io_service_(), test_timer_(io_service_), run_io_service_timer_(io_service_), - clients_(), num_threads_(), num_clients_(), num_in_progress_(0), num_finished_(0), - chunk_size_(0), paused_(false), pause_cnt_(0) { + : listener_(), io_service_(), test_timer_(io_service_), + run_io_service_timer_(io_service_), clients_(), num_threads_(), + num_clients_(), num_in_progress_(0), num_finished_(0), chunk_size_(0), + pause_cnt_(0) { test_timer_.setup(std::bind(&CmdHttpListenerTest::timeoutHandler, this, true), TEST_TIMEOUT, IntervalTimer::ONE_SHOT); @@ -181,10 +182,6 @@ public: /// @param request_limit Desired number of requests the function should wait /// to be processed before returning. void runIOService(size_t request_limit = 0) { - // Create a timer to use for invoking resume after pause. - IntervalTimer pause_timer_(io_service_); - paused_ = false; - if (!request_limit) { request_limit = clients_.size(); } @@ -720,9 +717,6 @@ public: /// @brief Condition variable used to coordinate threads. std::condition_variable cv_; - /// @brief Indicates if client threads are currently "paused". - bool paused_; - /// @brief Number of times client has been paused during the test. size_t pause_cnt_; }; @@ -847,7 +841,7 @@ TEST_F(CmdHttpListenerTest, basicListenAndRespond) { // to our listener, post our request and retrieve our reply. ASSERT_NO_THROW(startRequest("{\"command\": \"foo\"}")); ++num_clients_; - ASSERT_EQ(1, clients_.size()); + ASSERT_EQ(num_clients_, clients_.size()); ASSERT_NO_THROW(runIOService()); TestHttpClientPtr client = clients_.front(); ASSERT_TRUE(client); @@ -866,7 +860,7 @@ TEST_F(CmdHttpListenerTest, basicListenAndRespond) { // Try posting the foo command again. ASSERT_NO_THROW(startRequest("{\"command\": \"foo\"}")); ++num_clients_; - ASSERT_EQ(2, clients_.size()); + ASSERT_EQ(num_clients_, clients_.size()); ASSERT_NO_THROW(runIOService()); client = clients_.back(); ASSERT_TRUE(client); diff --git a/src/lib/http/client.cc b/src/lib/http/client.cc index 48df1b87b6..dcebd6012c 100644 --- a/src/lib/http/client.cc +++ b/src/lib/http/client.cc @@ -1778,14 +1778,14 @@ public: stop(); } - /// @brief Starts client's thread pool, if multi-threaded. + /// @brief Starts running the client's thread pool, if multi-threaded. void start() { if (thread_pool_) { thread_pool_->run(); } } - /// @brief Close all connections, and if multi-threaded, stops the + /// @brief Close all connections, and if multi-threaded, stops the client's /// thread pool. void stop() { // Close all the connections. @@ -1797,9 +1797,10 @@ public: } } - /// @brief Pauses the thread pool operation. + /// @brief Pauses the client's thread pool. /// /// Suspends thread pool event processing. + /// @throw InvalidOperation if the thread pool does not exist. void pause() { if (!thread_pool_) { isc_throw(InvalidOperation, "HttpClient::pause - no thread pool"); @@ -1809,9 +1810,10 @@ public: thread_pool_->pause(); } - /// @brief Resumes the thread pool operation. + /// @brief Resumes running the client's thread pool. /// /// Resumes thread pool event processing. + /// @throw InvalidOperation if the thread pool does not exist. void resume() { if (!thread_pool_) { isc_throw(InvalidOperation, "HttpClient::resume - no thread pool"); @@ -1821,9 +1823,9 @@ public: thread_pool_->run(); } - /// @brief Indicates if the thread pool processing is running. + /// @brief Indicates if the thread pool is running. /// - /// @return True if the thread pool exists and is in the RUNNING state, + /// @return True if the thread pool exists and it is in the RUNNING state, /// false otherwise. bool isRunning() { if (thread_pool_) { @@ -1835,8 +1837,8 @@ public: /// @brief Indicates if the thread pool is stopped. /// - /// @return True if the thread pool exists and is in the STOPPED state, - /// false otherwise + /// @return True if the thread pool exists and it is in the STOPPED state, + /// false otherwise. bool isStopped() { if (thread_pool_) { return (thread_pool_->isStopped()); @@ -1845,9 +1847,9 @@ public: return (false); } - /// @brief Indicates if the thread pool processing is running. + /// @brief Indicates if the thread pool is paused. /// - /// @return True if the thread pool exists and is in the PAUSED state, + /// @return True if the thread pool exists and it is in the PAUSED state, /// false otherwise. bool isPaused() { if (thread_pool_) { diff --git a/src/lib/http/client.h b/src/lib/http/client.h index bfe85ab031..aeb5fc340c 100644 --- a/src/lib/http/client.h +++ b/src/lib/http/client.h @@ -137,15 +137,15 @@ public: /// @param io_service IO service to be used by the HTTP client. /// @param thread_pool_size maximum number of threads in the thread pool. /// @param defer_thread_start if true, the thread pool will be created but - /// started. Applicable only when thread-pool-size is greater than zero. + /// not started. Applicable only when thread-pool-size is greater than zero. /// A value greater than zero enables multi-threaded mode and sets the /// maximum number of concurrent connections per URL. A value of zero /// (default) enables single-threaded mode with one connection per URL. - /// @param defer_thread_start When true, creation of the pool threads is + /// @param defer_thread_start When true, starting of the pool threads is /// deferred until a subsequent call to @ref start(). In this case the - /// pool's operational state post-construction is STOPPED. Otherwise, - /// the thread pool threads will be created and started, with the post- - /// construction state being RUNNING. Applicable only when thread-pool size + /// pool's operational state after construction is STOPPED. Otherwise, + /// the thread pool threads will be created and started, with the + /// operational state being RUNNING. Applicable only when thread-pool size /// is greater than zero. explicit HttpClient(asiolink::IOService& io_service, size_t thread_pool_size = 0, bool defer_thread_start = false); @@ -251,16 +251,16 @@ public: const CloseHandler& close_callback = CloseHandler()); - /// @brief Starts client's thread pool, if multi-threaded. + /// @brief Starts running the client's thread pool, if multi-threaded. void start(); - /// @brief Pauses the thread pool operation. + /// @brief Pauses the client's thread pool. /// /// Suspends thread pool event processing. /// @throw InvalidOperation if the thread pool does not exist. void pause(); - /// @brief Resumes the thread pool operation. + /// @brief Resumes running the client's thread pool. /// /// Resumes thread pool event processing. /// @throw InvalidOperation if the thread pool does not exist. @@ -306,19 +306,19 @@ public: /// @brief Indicates if the thread pool is running. /// - /// @return True if the thread pool exists and is in the RUNNING state, + /// @return True if the thread pool exists and it is in the RUNNING state, /// false otherwise. bool isRunning(); /// @brief Indicates if the thread pool is stopped. /// - /// @return True if the thread pool exists and is in the STOPPED state, + /// @return True if the thread pool exists and it is in the STOPPED state, /// false otherwise. bool isStopped(); - /// @brief Indicates if the thread pool processing is running. + /// @brief Indicates if the thread pool is paused. /// - /// @return True if the thread pool exists and is in the PAUSED state, + /// @return True if the thread pool exists and it is in the PAUSED state, /// false otherwise. bool isPaused(); diff --git a/src/lib/http/http_thread_pool.cc b/src/lib/http/http_thread_pool.cc index 288bb912f5..7e649f5f39 100644 --- a/src/lib/http/http_thread_pool.cc +++ b/src/lib/http/http_thread_pool.cc @@ -77,20 +77,16 @@ HttpThreadPool::getState() { bool HttpThreadPool::validateStateChange(State new_state) const { - bool is_valid = false; - switch(run_state_) { + switch (run_state_) { case State::STOPPED: - is_valid = (new_state == State::RUNNING); - break; + return (new_state == State::RUNNING); case State::RUNNING: - is_valid = (new_state != State::RUNNING); - break; + return (new_state != State::RUNNING); case State::PAUSED: - is_valid = (new_state != State::PAUSED); - break; + return (new_state != State::PAUSED); } - return (is_valid); + return (false); } void @@ -106,7 +102,7 @@ HttpThreadPool::setState(State new_state) { // Notify threads of state change. thread_cv_.notify_all(); - switch(new_state) { + switch (new_state) { case State::RUNNING: { // Restart the IOService. io_service_->restart(); diff --git a/src/lib/http/http_thread_pool.h b/src/lib/http/http_thread_pool.h index 8f7dba5914..e1d101728d 100644 --- a/src/lib/http/http_thread_pool.h +++ b/src/lib/http/http_thread_pool.h @@ -65,7 +65,7 @@ public: /// state. void pause(); - /// @brief Transitions the pool to from RUNNING OR PAUSED to STOPPED. + /// @brief Transitions the pool from RUNNING or PAUSED to STOPPED. /// /// Stops thread event processing and then destroys the pool's threads /// Has no effect if the pool is already in the STOPPED state. @@ -103,7 +103,7 @@ private: /// -# Restarts the IOService. /// -# Creates the threads if they do not yet exist (true only /// when transitioning from STOPPED). - /// -# Waits until threads are running. + /// -# Waits until all threads are running. /// -# Sets the count of exited threads to 0. /// -# Returns to caller. /// @@ -167,14 +167,13 @@ private: /// -# The count of threads exited is incremented. /// -# If the count has reached the number of threads in pool the /// main thread is notified. - /// -# function exits. + /// -# The function exits. void threadWork(); public: - /// @brief Fetches the IOService that drives the pool. /// - /// @return A pointer to the IOService. + /// @return the pointer to the IOService. asiolink::IOServicePtr getIOService() const; /// @brief Fetches the maximum size of the thread pool. diff --git a/src/lib/http/tests/mt_client_unittests.cc b/src/lib/http/tests/mt_client_unittests.cc index ed7adefc6f..2dfc69a251 100644 --- a/src/lib/http/tests/mt_client_unittests.cc +++ b/src/lib/http/tests/mt_client_unittests.cc @@ -313,7 +313,7 @@ public: } else { // I'm ready but others aren't wait here. bool ret = test_cv_.wait_for(lck, std::chrono::seconds(10), - [&]() { return (num_in_progress_ == num_threads_); }); + [&]() { return (num_in_progress_ == num_threads_); }); if (!ret) { ADD_FAILURE() << "clients failed to start work"; } @@ -344,7 +344,7 @@ public: } else { // I'm done but others aren't wait here. bool ret = test_cv_.wait_for(lck, std::chrono::seconds(10), - [&]() { return (num_finished_ == num_threads_); }); + [&]() { return (num_finished_ == num_threads_); }); if (!ret) { ADD_FAILURE() << "clients failed to finish work"; } @@ -353,7 +353,6 @@ public: })); } - /// @brief Initiates a single HTTP request. /// /// Constructs an HTTP post whose body is a JSON map containing a @@ -772,8 +771,6 @@ public: /// @brief Number of times client has been paused during the test. size_t pause_cnt_; - - bool shutdown_; }; // Verifies we can construct and destruct, in both single