From: Razvan Becheriu Date: Fri, 27 Aug 2021 18:40:34 +0000 (+0300) Subject: [#2043] allow callbacks to create CS inside the CS constructor and destructor X-Git-Tag: Kea-1.9.11~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4d9cfe01e8a8e049c4ceb0ff12fe8e7976cb2f71;p=thirdparty%2Fkea.git [#2043] allow callbacks to create CS inside the CS constructor and destructor --- diff --git a/src/lib/util/multi_threading_mgr.cc b/src/lib/util/multi_threading_mgr.cc index c6689667cc..7efd048bca 100644 --- a/src/lib/util/multi_threading_mgr.cc +++ b/src/lib/util/multi_threading_mgr.cc @@ -37,30 +37,45 @@ MultiThreadingMgr::setMode(bool enabled) { void MultiThreadingMgr::enterCriticalSection() { checkCallbacksPermissions(); - std::lock_guard lk(mutex_); - stopProcessing(); + bool inside = isInCriticalSection(); + // Increment the counter to allow CS to be created in the registered + // callbacks (in which case the new CS would not call callbacks again). + // The counter must be updated regardless of the MT mode because the MT mode + // can change between the constructor call and the destructor call. ++critical_section_count_; + if (getMode() && !inside) { + if (getThreadPoolSize()) { + thread_pool_.stop(); + } + // Now it is safe to call callbacks which can also create other CSs. + callEntryCallbacks(); + } } void MultiThreadingMgr::exitCriticalSection() { - std::lock_guard lk(mutex_); - if (critical_section_count_ == 0) { - isc_throw(InvalidOperation, "invalid negative value for override"); + // The number of CS destructors should match the number of CS constructors. + // The case when counter is 0 is only possible if calling this function + // explicitly, which is a programming error. + if (!isInCriticalSection()) { + isc_throw(InvalidOperation, "invalid value for critical section count"); } + // Decrement the counter to allow the check for last CS destructor which + // would result in restarting the thread pool. + // The counter must be updated regardless of the MT mode because the MT mode + // can change between the constructor call and the destructor call. --critical_section_count_; - startProcessing(); + if (getMode() && !isInCriticalSection()) { + if (getThreadPoolSize()) { + thread_pool_.start(getThreadPoolSize()); + } + // Now it is safe to call callbacks which can also create other CSs. + callExitCallbacks(); + } } bool MultiThreadingMgr::isInCriticalSection() { - checkCallbacksPermissions(); - std::lock_guard lk(mutex_); - return (isInCriticalSectionInternal()); -} - -bool -MultiThreadingMgr::isInCriticalSectionInternal() { return (critical_section_count_ != 0); } @@ -186,26 +201,6 @@ MultiThreadingMgr::callExitCallbacks() { } } -void -MultiThreadingMgr::stopProcessing() { - if (getMode() && !isInCriticalSectionInternal()) { - if (getThreadPoolSize()) { - thread_pool_.stop(); - } - callEntryCallbacks(); - } -} - -void -MultiThreadingMgr::startProcessing() { - if (getMode() && !isInCriticalSectionInternal()) { - if (getThreadPoolSize()) { - thread_pool_.start(getThreadPoolSize()); - } - callExitCallbacks(); - } -} - void MultiThreadingMgr::addCriticalSectionCallbacks(const std::string& name, const CSCallbackSet::Callback& check_cb, diff --git a/src/lib/util/multi_threading_mgr.h b/src/lib/util/multi_threading_mgr.h index ec2d991960..fa3f3a353e 100644 --- a/src/lib/util/multi_threading_mgr.h +++ b/src/lib/util/multi_threading_mgr.h @@ -159,6 +159,12 @@ public: /// counter so that any configuration change that might start the packet /// thread pool is delayed until exiting the respective section. /// If the internal counter is 0, then stop the thread pool. + /// + /// Invokes all CriticalSection entry callbacks. Has no effect in + /// single-threaded mode. + /// + /// @note This function swallows exceptions thrown by all entry callbacks + /// without logging to avoid breaking the CS chain. void enterCriticalSection(); /// @brief Exit critical section. @@ -167,6 +173,12 @@ public: /// counter so that the dhcp thread pool can be started according to the /// new configuration. /// If the internal counter is 0, then start the thread pool. + /// + /// Invokes all CriticalSection exit callbacks. Has no effect in + /// single-threaded mode. + /// + /// @note This function swallows exceptions thrown by all exit callbacks + /// without logging to avoid breaking the CS chain. void exitCriticalSection(); /// @brief Is in critical section flag. @@ -259,13 +271,6 @@ protected: private: - /// @brief Is in critical section flag. - /// - /// Should be called in a thread safe context. - /// - /// @return The critical section flag. - bool isInCriticalSectionInternal(); - /// @brief Class method tests if current thread is allowed to enter the /// @ref MultiThreadingCriticalSection and to invoke the entry and exit /// callbacks. @@ -296,25 +301,6 @@ private: /// without logging to avoid breaking the CS chain. void callExitCallbacks(); - /// @brief Class method stops non-critical processing. - /// - /// Stops the DHCP thread pool if it's running and invokes all - /// CriticalSection entry callbacks. Has no effect in single-threaded mode. - /// - /// @note This function swallows exceptions thrown by all exit callbacks - /// without logging to avoid breaking the CS chain. - void stopProcessing(); - - /// @brief Class method (re)starts non-critical processing. - /// - /// Starts the DHCP thread pool according to current configuration, and - /// invokes all CriticalSection exit callbacks. Has no effect in - /// single-threaded mode. - /// - /// @note This function swallows exceptions thrown by all entry callbacks - /// without logging to avoid breaking the CS chain. - void startProcessing(); - /// @brief The current multi-threading mode. /// /// The multi-threading flag: true if multi-threading is enabled, false @@ -337,9 +323,6 @@ private: /// @brief List of CriticalSection entry and exit callback sets. CSCallbackSetList cs_callbacks_; - - /// @brief Mutex to protect the internal state. - std::mutex mutex_; }; /// @note: everything here MUST be used ONLY from the main thread.