From 0fb4e4d642054164c0e3227575b87cbe745ed4f9 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Fri, 8 Nov 2024 11:47:23 +0100 Subject: [PATCH 01/11] Remove unused Registry#OnUnregistered --- lib/base/registry.hpp | 9 --------- 1 file changed, 9 deletions(-) diff --git a/lib/base/registry.hpp b/lib/base/registry.hpp index 5a8ac26f7..7c73c2084 100644 --- a/lib/base/registry.hpp +++ b/lib/base/registry.hpp @@ -50,7 +50,6 @@ public: } boost::signals2::signal OnRegistered; - boost::signals2::signal OnUnregistered; private: mutable std::mutex m_Mutex; @@ -58,18 +57,10 @@ private: void RegisterInternal(const String& name, const T& item, std::unique_lock& lock) { - bool old_item = false; - - if (m_Items.erase(name) > 0) - old_item = true; - m_Items[name] = item; lock.unlock(); - if (old_item) - OnUnregistered(name); - OnRegistered(name, item); } }; From 87a2f9fadec363831df2165c85c534827c983076 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Fri, 8 Nov 2024 11:48:15 +0100 Subject: [PATCH 02/11] Remove unused Registry#OnRegistered --- lib/base/registry.hpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lib/base/registry.hpp b/lib/base/registry.hpp index 7c73c2084..ba0952ee6 100644 --- a/lib/base/registry.hpp +++ b/lib/base/registry.hpp @@ -5,7 +5,6 @@ #include "base/i2-base.hpp" #include "base/string.hpp" -#include #include #include @@ -49,8 +48,6 @@ public: return m_Items; /* Makes a copy of the map. */ } - boost::signals2::signal OnRegistered; - private: mutable std::mutex m_Mutex; typename Registry::ItemMap m_Items; @@ -60,8 +57,6 @@ private: m_Items[name] = item; lock.unlock(); - - OnRegistered(name, item); } }; From c3d97271335ee8c0f00a6ded0139bafa1d9199df Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Fri, 8 Nov 2024 11:50:31 +0100 Subject: [PATCH 03/11] Inline Registry#RegisterInternal() used only once --- lib/base/registry.hpp | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/lib/base/registry.hpp b/lib/base/registry.hpp index ba0952ee6..6133da525 100644 --- a/lib/base/registry.hpp +++ b/lib/base/registry.hpp @@ -26,7 +26,7 @@ public: { std::unique_lock lock(m_Mutex); - RegisterInternal(name, item, lock); + m_Items[name] = item; } T GetItem(const String& name) const @@ -51,13 +51,6 @@ public: private: mutable std::mutex m_Mutex; typename Registry::ItemMap m_Items; - - void RegisterInternal(const String& name, const T& item, std::unique_lock& lock) - { - m_Items[name] = item; - - lock.unlock(); - } }; } From 7fc722e581962c428ce9c58c027df34c99384395 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Fri, 8 Nov 2024 11:53:27 +0100 Subject: [PATCH 04/11] Make Registry#ItemMap a hash table to speed up lookups --- lib/base/registry.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/base/registry.hpp b/lib/base/registry.hpp index 6133da525..5fa73e43f 100644 --- a/lib/base/registry.hpp +++ b/lib/base/registry.hpp @@ -5,7 +5,7 @@ #include "base/i2-base.hpp" #include "base/string.hpp" -#include +#include #include namespace icinga @@ -20,7 +20,7 @@ template class Registry { public: - typedef std::map ItemMap; + typedef std::unordered_map ItemMap; void Register(const String& name, const T& item) { From 496a65d0d054262c1af7b5529b800601452b906c Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Fri, 8 Nov 2024 11:57:05 +0100 Subject: [PATCH 05/11] Registry#Get*(): use shared locking to allow concurrent access --- lib/base/registry.hpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/base/registry.hpp b/lib/base/registry.hpp index 5fa73e43f..9380b4705 100644 --- a/lib/base/registry.hpp +++ b/lib/base/registry.hpp @@ -5,6 +5,7 @@ #include "base/i2-base.hpp" #include "base/string.hpp" +#include #include #include @@ -24,14 +25,14 @@ public: void Register(const String& name, const T& item) { - std::unique_lock lock(m_Mutex); + std::unique_lock lock (m_Mutex); m_Items[name] = item; } T GetItem(const String& name) const { - std::unique_lock lock(m_Mutex); + std::shared_lock lock (m_Mutex); auto it = m_Items.find(name); @@ -43,13 +44,13 @@ public: ItemMap GetItems() const { - std::unique_lock lock(m_Mutex); + std::shared_lock lock (m_Mutex); return m_Items; /* Makes a copy of the map. */ } private: - mutable std::mutex m_Mutex; + mutable std::shared_mutex m_Mutex; typename Registry::ItemMap m_Items; }; From 2e3551e497bcd475a59f5019dcb60d28bf70b8c9 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Fri, 8 Nov 2024 12:18:38 +0100 Subject: [PATCH 06/11] Introduce Registry#Freeze() --- lib/base/registry.hpp | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/lib/base/registry.hpp b/lib/base/registry.hpp index 9380b4705..f60c3490a 100644 --- a/lib/base/registry.hpp +++ b/lib/base/registry.hpp @@ -4,8 +4,11 @@ #define REGISTRY_H #include "base/i2-base.hpp" +#include "base/atomic.hpp" +#include "base/exception.hpp" #include "base/string.hpp" #include +#include #include #include @@ -27,12 +30,16 @@ public: { std::unique_lock lock (m_Mutex); + if (m_Frozen) { + BOOST_THROW_EXCEPTION(std::logic_error("Registry is read-only and must not be modified.")); + } + m_Items[name] = item; } T GetItem(const String& name) const { - std::shared_lock lock (m_Mutex); + auto lock (ReadLockUnlessFrozen()); auto it = m_Items.find(name); @@ -44,14 +51,37 @@ public: ItemMap GetItems() const { - std::shared_lock lock (m_Mutex); + auto lock (ReadLockUnlessFrozen()); return m_Items; /* Makes a copy of the map. */ } + /** + * Freeze the registry, preventing further updates. + * + * This only prevents inserting, replacing or deleting values from the registry. + * This operation has no effect on objects referenced by the values, these remain mutable if they were before. + */ + void Freeze() + { + std::unique_lock lock (m_Mutex); + + m_Frozen.store(true); + } + private: mutable std::shared_mutex m_Mutex; + Atomic m_Frozen {false}; typename Registry::ItemMap m_Items; + + std::shared_lock ReadLockUnlessFrozen() const + { + if (m_Frozen.load(std::memory_order_relaxed)) { + return {}; + } + + return std::shared_lock(m_Mutex); + } }; } From 68a84802517029dfd89d0476ccf0597181a2c55a Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Fri, 8 Nov 2024 12:43:08 +0100 Subject: [PATCH 07/11] Introduce Registry::GetInstance() to deduplicate such methods in derived classes and inline them, as side effect, to speed up calls. --- lib/base/registry.hpp | 6 ++++++ lib/db_ido/dbtype.cpp | 5 ----- lib/db_ido/dbtype.hpp | 3 --- lib/remote/apiaction.cpp | 6 ------ lib/remote/apiaction.hpp | 2 -- lib/remote/apifunction.cpp | 6 ------ lib/remote/apifunction.hpp | 2 -- lib/remote/eventqueue.cpp | 6 ------ lib/remote/eventqueue.hpp | 2 -- 9 files changed, 6 insertions(+), 32 deletions(-) diff --git a/lib/base/registry.hpp b/lib/base/registry.hpp index f60c3490a..42985f6ce 100644 --- a/lib/base/registry.hpp +++ b/lib/base/registry.hpp @@ -7,6 +7,7 @@ #include "base/atomic.hpp" #include "base/exception.hpp" #include "base/string.hpp" +#include "base/singleton.hpp" #include #include #include @@ -26,6 +27,11 @@ class Registry public: typedef std::unordered_map ItemMap; + static Registry* GetInstance() + { + return Singleton::GetInstance(); + } + void Register(const String& name, const T& item) { std::unique_lock lock (m_Mutex); diff --git a/lib/db_ido/dbtype.cpp b/lib/db_ido/dbtype.cpp index c5476b179..8006171ce 100644 --- a/lib/db_ido/dbtype.cpp +++ b/lib/db_ido/dbtype.cpp @@ -133,8 +133,3 @@ std::set DbType::GetAllTypes() return result; } - -DbTypeRegistry *DbTypeRegistry::GetInstance() -{ - return Singleton::GetInstance(); -} diff --git a/lib/db_ido/dbtype.hpp b/lib/db_ido/dbtype.hpp index c8ebc4504..d4cf82122 100644 --- a/lib/db_ido/dbtype.hpp +++ b/lib/db_ido/dbtype.hpp @@ -6,7 +6,6 @@ #include "db_ido/i2-db_ido.hpp" #include "base/object.hpp" #include "base/registry.hpp" -#include "base/singleton.hpp" #include namespace icinga @@ -64,8 +63,6 @@ private: */ class DbTypeRegistry : public Registry { -public: - static DbTypeRegistry *GetInstance(); }; /** diff --git a/lib/remote/apiaction.cpp b/lib/remote/apiaction.cpp index ccde28190..d2cf96897 100644 --- a/lib/remote/apiaction.cpp +++ b/lib/remote/apiaction.cpp @@ -1,7 +1,6 @@ /* Icinga 2 | (c) 2012 Icinga GmbH | GPLv2+ */ #include "remote/apiaction.hpp" -#include "base/singleton.hpp" using namespace icinga; @@ -28,8 +27,3 @@ void ApiAction::Register(const String& name, const ApiAction::Ptr& action) { ApiActionRegistry::GetInstance()->Register(name, action); } - -ApiActionRegistry *ApiActionRegistry::GetInstance() -{ - return Singleton::GetInstance(); -} diff --git a/lib/remote/apiaction.hpp b/lib/remote/apiaction.hpp index 2bb98b1b6..d0db391a0 100644 --- a/lib/remote/apiaction.hpp +++ b/lib/remote/apiaction.hpp @@ -47,8 +47,6 @@ private: */ class ApiActionRegistry : public Registry { -public: - static ApiActionRegistry *GetInstance(); }; #define REGISTER_APIACTION(name, types, callback) \ diff --git a/lib/remote/apifunction.cpp b/lib/remote/apifunction.cpp index f153dcb46..166b0fd1a 100644 --- a/lib/remote/apifunction.cpp +++ b/lib/remote/apifunction.cpp @@ -1,7 +1,6 @@ /* Icinga 2 | (c) 2012 Icinga GmbH | GPLv2+ */ #include "remote/apifunction.hpp" -#include "base/singleton.hpp" using namespace icinga; @@ -23,8 +22,3 @@ void ApiFunction::Register(const String& name, const ApiFunction::Ptr& function) { ApiFunctionRegistry::GetInstance()->Register(name, function); } - -ApiFunctionRegistry *ApiFunctionRegistry::GetInstance() -{ - return Singleton::GetInstance(); -} diff --git a/lib/remote/apifunction.hpp b/lib/remote/apifunction.hpp index ea8b7cf1e..3c76a5735 100644 --- a/lib/remote/apifunction.hpp +++ b/lib/remote/apifunction.hpp @@ -49,8 +49,6 @@ private: */ class ApiFunctionRegistry : public Registry { -public: - static ApiFunctionRegistry *GetInstance(); }; #define REGISTER_APIFUNCTION(name, ns, callback) \ diff --git a/lib/remote/eventqueue.cpp b/lib/remote/eventqueue.cpp index 819f95a6a..fa7ab0c16 100644 --- a/lib/remote/eventqueue.cpp +++ b/lib/remote/eventqueue.cpp @@ -4,7 +4,6 @@ #include "remote/eventqueue.hpp" #include "remote/filterutility.hpp" #include "base/io-engine.hpp" -#include "base/singleton.hpp" #include "base/logger.hpp" #include "base/utility.hpp" #include @@ -125,11 +124,6 @@ void EventQueue::Register(const String& name, const EventQueue::Ptr& function) EventQueueRegistry::GetInstance()->Register(name, function); } -EventQueueRegistry *EventQueueRegistry::GetInstance() -{ - return Singleton::GetInstance(); -} - std::mutex EventsInbox::m_FiltersMutex; std::map EventsInbox::m_Filters ({{"", EventsInbox::Filter{1, Expression::Ptr()}}}); diff --git a/lib/remote/eventqueue.hpp b/lib/remote/eventqueue.hpp index 833714f9d..2836ec140 100644 --- a/lib/remote/eventqueue.hpp +++ b/lib/remote/eventqueue.hpp @@ -61,8 +61,6 @@ private: */ class EventQueueRegistry : public Registry { -public: - static EventQueueRegistry *GetInstance(); }; enum class EventType : uint_fast8_t From cba30e7d0548ce184b2b36b59a72d8e2622ed525 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Fri, 8 Nov 2024 13:15:59 +0100 Subject: [PATCH 08/11] Actually use Registry#Freeze() at startup, when everything has been registered --- lib/db_ido/dbtype.cpp | 4 ++++ lib/remote/apiaction.cpp | 4 ++++ lib/remote/apifunction.cpp | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/lib/db_ido/dbtype.cpp b/lib/db_ido/dbtype.cpp index 8006171ce..06b5ccd2a 100644 --- a/lib/db_ido/dbtype.cpp +++ b/lib/db_ido/dbtype.cpp @@ -8,6 +8,10 @@ using namespace icinga; +INITIALIZE_ONCE_WITH_PRIORITY([]{ + DbTypeRegistry::GetInstance()->Freeze(); +}, InitializePriority::FreezeNamespaces); + DbType::DbType(String name, String table, long tid, String idcolumn, DbType::ObjectFactory factory) : m_Name(std::move(name)), m_Table(std::move(table)), m_TypeID(tid), m_IDColumn(std::move(idcolumn)), m_ObjectFactory(std::move(factory)) { } diff --git a/lib/remote/apiaction.cpp b/lib/remote/apiaction.cpp index d2cf96897..ae31581d5 100644 --- a/lib/remote/apiaction.cpp +++ b/lib/remote/apiaction.cpp @@ -4,6 +4,10 @@ using namespace icinga; +INITIALIZE_ONCE_WITH_PRIORITY([]{ + ApiActionRegistry::GetInstance()->Freeze(); +}, InitializePriority::FreezeNamespaces); + ApiAction::ApiAction(std::vector types, Callback action) : m_Types(std::move(types)), m_Callback(std::move(action)) { } diff --git a/lib/remote/apifunction.cpp b/lib/remote/apifunction.cpp index 166b0fd1a..563a5519f 100644 --- a/lib/remote/apifunction.cpp +++ b/lib/remote/apifunction.cpp @@ -4,6 +4,10 @@ using namespace icinga; +INITIALIZE_ONCE_WITH_PRIORITY([]{ + ApiFunctionRegistry::GetInstance()->Freeze(); +}, InitializePriority::FreezeNamespaces); + ApiFunction::ApiFunction(const char* name, Callback function) : m_Name(name), m_Callback(std::move(function)) { } From 74ac0183cad8ae9fae8d6662071dfac4cbeb9069 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Mon, 26 Jan 2026 10:24:22 +0100 Subject: [PATCH 09/11] `Registry`: remove unused template typename `U` --- lib/base/registry.hpp | 4 ++-- lib/db_ido/dbtype.hpp | 2 +- lib/remote/apiaction.hpp | 2 +- lib/remote/apifunction.hpp | 2 +- lib/remote/eventqueue.hpp | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/base/registry.hpp b/lib/base/registry.hpp index 42985f6ce..e2ef6784f 100644 --- a/lib/base/registry.hpp +++ b/lib/base/registry.hpp @@ -21,7 +21,7 @@ namespace icinga * * @ingroup base */ -template +template class Registry { public: @@ -78,7 +78,7 @@ public: private: mutable std::shared_mutex m_Mutex; Atomic m_Frozen {false}; - typename Registry::ItemMap m_Items; + ItemMap m_Items; std::shared_lock ReadLockUnlessFrozen() const { diff --git a/lib/db_ido/dbtype.hpp b/lib/db_ido/dbtype.hpp index d4cf82122..534062218 100644 --- a/lib/db_ido/dbtype.hpp +++ b/lib/db_ido/dbtype.hpp @@ -61,7 +61,7 @@ private: * * @ingroup ido */ -class DbTypeRegistry : public Registry +class DbTypeRegistry : public Registry { }; diff --git a/lib/remote/apiaction.hpp b/lib/remote/apiaction.hpp index d0db391a0..2ce8ff153 100644 --- a/lib/remote/apiaction.hpp +++ b/lib/remote/apiaction.hpp @@ -45,7 +45,7 @@ private: * * @ingroup remote */ -class ApiActionRegistry : public Registry +class ApiActionRegistry : public Registry { }; diff --git a/lib/remote/apifunction.hpp b/lib/remote/apifunction.hpp index 3c76a5735..b9cee1eee 100644 --- a/lib/remote/apifunction.hpp +++ b/lib/remote/apifunction.hpp @@ -47,7 +47,7 @@ private: * * @ingroup base */ -class ApiFunctionRegistry : public Registry +class ApiFunctionRegistry : public Registry { }; diff --git a/lib/remote/eventqueue.hpp b/lib/remote/eventqueue.hpp index 2836ec140..da4737b04 100644 --- a/lib/remote/eventqueue.hpp +++ b/lib/remote/eventqueue.hpp @@ -59,7 +59,7 @@ private: * * @ingroup base */ -class EventQueueRegistry : public Registry +class EventQueueRegistry : public Registry { }; From b4192bd80a205581635bfe590a58fa0ba3f4bd3e Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Mon, 26 Jan 2026 14:34:29 +0100 Subject: [PATCH 10/11] Replace `class B : public A { };` with `using B = A;` (refactor only) --- lib/db_ido/dbtype.hpp | 4 +--- lib/remote/apiaction.hpp | 4 +--- lib/remote/apifunction.hpp | 4 +--- lib/remote/eventqueue.hpp | 4 +--- 4 files changed, 4 insertions(+), 12 deletions(-) diff --git a/lib/db_ido/dbtype.hpp b/lib/db_ido/dbtype.hpp index 534062218..68fe25152 100644 --- a/lib/db_ido/dbtype.hpp +++ b/lib/db_ido/dbtype.hpp @@ -61,9 +61,7 @@ private: * * @ingroup ido */ -class DbTypeRegistry : public Registry -{ -}; +using DbTypeRegistry = Registry; /** * Factory function for DbObject-based classes. diff --git a/lib/remote/apiaction.hpp b/lib/remote/apiaction.hpp index 2ce8ff153..8d803d04f 100644 --- a/lib/remote/apiaction.hpp +++ b/lib/remote/apiaction.hpp @@ -45,9 +45,7 @@ private: * * @ingroup remote */ -class ApiActionRegistry : public Registry -{ -}; +using ApiActionRegistry = Registry; #define REGISTER_APIACTION(name, types, callback) \ INITIALIZE_ONCE([]() { \ diff --git a/lib/remote/apifunction.hpp b/lib/remote/apifunction.hpp index b9cee1eee..2b0f3ed00 100644 --- a/lib/remote/apifunction.hpp +++ b/lib/remote/apifunction.hpp @@ -47,9 +47,7 @@ private: * * @ingroup base */ -class ApiFunctionRegistry : public Registry -{ -}; +using ApiFunctionRegistry = Registry; #define REGISTER_APIFUNCTION(name, ns, callback) \ INITIALIZE_ONCE([]() { \ diff --git a/lib/remote/eventqueue.hpp b/lib/remote/eventqueue.hpp index da4737b04..7b8b7e941 100644 --- a/lib/remote/eventqueue.hpp +++ b/lib/remote/eventqueue.hpp @@ -59,9 +59,7 @@ private: * * @ingroup base */ -class EventQueueRegistry : public Registry -{ -}; +using EventQueueRegistry = Registry; enum class EventType : uint_fast8_t { From 6223bd5f317ecb8121c83c687dfa658cc0f58632 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Mon, 26 Jan 2026 14:37:59 +0100 Subject: [PATCH 11/11] Remove unused `DbTypeRegistry` --- lib/db_ido/dbtype.cpp | 4 ---- lib/db_ido/dbtype.hpp | 7 ------- 2 files changed, 11 deletions(-) diff --git a/lib/db_ido/dbtype.cpp b/lib/db_ido/dbtype.cpp index 06b5ccd2a..8006171ce 100644 --- a/lib/db_ido/dbtype.cpp +++ b/lib/db_ido/dbtype.cpp @@ -8,10 +8,6 @@ using namespace icinga; -INITIALIZE_ONCE_WITH_PRIORITY([]{ - DbTypeRegistry::GetInstance()->Freeze(); -}, InitializePriority::FreezeNamespaces); - DbType::DbType(String name, String table, long tid, String idcolumn, DbType::ObjectFactory factory) : m_Name(std::move(name)), m_Table(std::move(table)), m_TypeID(tid), m_IDColumn(std::move(idcolumn)), m_ObjectFactory(std::move(factory)) { } diff --git a/lib/db_ido/dbtype.hpp b/lib/db_ido/dbtype.hpp index 68fe25152..b210aabb3 100644 --- a/lib/db_ido/dbtype.hpp +++ b/lib/db_ido/dbtype.hpp @@ -56,13 +56,6 @@ private: ObjectMap m_Objects; }; -/** - * A registry for DbType objects. - * - * @ingroup ido - */ -using DbTypeRegistry = Registry; - /** * Factory function for DbObject-based classes. *