From a66ace72455f629c047e3ed8805a24946f95e802 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Fri, 15 Jul 2022 15:17:37 +0200 Subject: [PATCH 1/3] Introduce SharedMemory --- lib/base/CMakeLists.txt | 1 + lib/base/shared-memory.hpp | 45 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 lib/base/shared-memory.hpp diff --git a/lib/base/CMakeLists.txt b/lib/base/CMakeLists.txt index 18e884de2..e50e330e4 100644 --- a/lib/base/CMakeLists.txt +++ b/lib/base/CMakeLists.txt @@ -65,6 +65,7 @@ set(base_SOURCES scriptutils.cpp scriptutils.hpp serializer.cpp serializer.hpp shared.hpp + shared-memory.hpp shared-object.hpp singleton.hpp socket.cpp socket.hpp diff --git a/lib/base/shared-memory.hpp b/lib/base/shared-memory.hpp new file mode 100644 index 000000000..dd350c89f --- /dev/null +++ b/lib/base/shared-memory.hpp @@ -0,0 +1,45 @@ +/* Icinga 2 | (c) 2023 Icinga GmbH | GPLv2+ */ + +#pragma once + +#include +#include + +namespace icinga +{ + +/** + * Type-safe memory shared across fork(2). + * + * @ingroup base + */ +template +class SharedMemory +{ +public: + template + SharedMemory(Args&&... args) : m_Memory(boost::interprocess::anonymous_shared_memory(sizeof(T))) + { + new(GetAddress()) T(std::forward(args)...); + } + + SharedMemory(const SharedMemory&) = delete; + SharedMemory(SharedMemory&&) = delete; + SharedMemory& operator=(const SharedMemory&) = delete; + SharedMemory& operator=(SharedMemory&&) = delete; + + inline T& Get() const + { + return *GetAddress(); + } + +private: + inline T* GetAddress() const + { + return (T*)m_Memory.get_address(); + } + + boost::interprocess::mapped_region m_Memory; +}; + +} From 64e000df562cd1908b71a8e05388b0b58fc1a8ba Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Fri, 15 Jul 2022 17:00:50 +0200 Subject: [PATCH 2/3] Introduce ConfigObjects*Lock --- lib/remote/CMakeLists.txt | 1 + lib/remote/configobjectslock.cpp | 24 +++++++++++ lib/remote/configobjectslock.hpp | 72 ++++++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+) create mode 100644 lib/remote/configobjectslock.cpp create mode 100644 lib/remote/configobjectslock.hpp diff --git a/lib/remote/CMakeLists.txt b/lib/remote/CMakeLists.txt index 2c5a0326a..740b112b4 100644 --- a/lib/remote/CMakeLists.txt +++ b/lib/remote/CMakeLists.txt @@ -14,6 +14,7 @@ set(remote_SOURCES apilistener-authority.cpp apiuser.cpp apiuser.hpp apiuser-ti.hpp configfileshandler.cpp configfileshandler.hpp + configobjectslock.cpp configobjectslock.hpp configobjectutility.cpp configobjectutility.hpp configpackageshandler.cpp configpackageshandler.hpp configpackageutility.cpp configpackageutility.hpp diff --git a/lib/remote/configobjectslock.cpp b/lib/remote/configobjectslock.cpp new file mode 100644 index 000000000..e529c832b --- /dev/null +++ b/lib/remote/configobjectslock.cpp @@ -0,0 +1,24 @@ +/* Icinga 2 | (c) 2022 Icinga GmbH | GPLv2+ */ + +#ifndef _WIN32 + +#include "base/shared-memory.hpp" +#include "remote/configobjectslock.hpp" +#include + +using namespace icinga; + +// On *nix one process may write config objects while another is loading the config, so this uses IPC. +static SharedMemory l_ConfigObjectsMutex; + +ConfigObjectsExclusiveLock::ConfigObjectsExclusiveLock() + : m_Lock(l_ConfigObjectsMutex.Get()) +{ +} + +ConfigObjectsSharedLock::ConfigObjectsSharedLock(std::try_to_lock_t) + : m_Lock(l_ConfigObjectsMutex.Get(), boost::interprocess::try_to_lock) +{ +} + +#endif /* _WIN32 */ diff --git a/lib/remote/configobjectslock.hpp b/lib/remote/configobjectslock.hpp new file mode 100644 index 000000000..ee909815f --- /dev/null +++ b/lib/remote/configobjectslock.hpp @@ -0,0 +1,72 @@ +/* Icinga 2 | (c) 2023 Icinga GmbH | GPLv2+ */ + +#pragma once + +#include + +#ifndef _WIN32 +#include +#include +#include +#endif /* _WIN32 */ + +namespace icinga +{ + +#ifdef _WIN32 + +class ConfigObjectsSharedLock +{ +public: + inline ConfigObjectsSharedLock(std::try_to_lock_t) + { + } + + constexpr explicit operator bool() const + { + return true; + } +}; + +#else /* _WIN32 */ + +/** + * Waits until all ConfigObjects*Lock-s have vanished. For its lifetime disallows such. + * Keep an instance alive during reload to forbid runtime config changes! + * This way Icinga reads a consistent config which doesn't suddenly get runtime-changed. + * + * @ingroup remote + */ +class ConfigObjectsExclusiveLock +{ +public: + ConfigObjectsExclusiveLock(); + +private: + boost::interprocess::scoped_lock m_Lock; +}; + +/** + * Waits until the only ConfigObjectsExclusiveLock has vanished (if any). For its lifetime disallows such. + * Keep an instance alive during runtime config changes to delay a reload (if any)! + * This way Icinga reads a consistent config which doesn't suddenly get runtime-changed. + * + * @ingroup remote + */ +class ConfigObjectsSharedLock +{ +public: + ConfigObjectsSharedLock(std::try_to_lock_t); + + inline explicit operator bool() const + { + return m_Lock.owns(); + } + +private: + boost::interprocess::sharable_lock m_Lock; +}; + +#endif /* _WIN32 */ + +} From 2ee776b5ab5a3f20553270fb93ba0e995ce55010 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Wed, 12 Apr 2023 14:45:40 +0200 Subject: [PATCH 3/3] Disallow config modifications via API during reload Once the new main process has read the config, it misses subsequent modifications from the old process otherwise. --- lib/cli/daemoncommand.cpp | 5 ++++ lib/icinga/apiactions.cpp | 37 ++++++++++++++++++++++++++++++ lib/remote/createobjecthandler.cpp | 8 +++++++ lib/remote/deleteobjecthandler.cpp | 8 +++++++ lib/remote/modifyobjecthandler.cpp | 8 +++++++ 5 files changed, 66 insertions(+) diff --git a/lib/cli/daemoncommand.cpp b/lib/cli/daemoncommand.cpp index 28ab0459d..1fca07b5c 100644 --- a/lib/cli/daemoncommand.cpp +++ b/lib/cli/daemoncommand.cpp @@ -3,6 +3,7 @@ #include "cli/daemoncommand.hpp" #include "cli/daemonutility.hpp" #include "remote/apilistener.hpp" +#include "remote/configobjectslock.hpp" #include "remote/configobjectutility.hpp" #include "config/configcompiler.hpp" #include "config/configcompilercontext.hpp" @@ -801,6 +802,10 @@ int DaemonCommand::Run(const po::variables_map& vm, const std::vectorGetName() + " is already acknowledged."); } + ConfigObjectsSharedLock lock (std::try_to_lock); + + if (!lock) { + return ApiActions::CreateResult(503, "Icinga is reloading."); + } + Comment::AddComment(checkable, CommentAcknowledgement, HttpUtility::GetLastParameter(params, "author"), HttpUtility::GetLastParameter(params, "comment"), persistent, timestamp, sticky == AcknowledgementSticky); checkable->AcknowledgeProblem(HttpUtility::GetLastParameter(params, "author"), @@ -272,6 +279,12 @@ Dictionary::Ptr ApiActions::RemoveAcknowledgement(const ConfigObject::Ptr& objec "Cannot remove acknowledgement for non-existent checkable object " + object->GetName() + "."); + ConfigObjectsSharedLock lock (std::try_to_lock); + + if (!lock) { + return ApiActions::CreateResult(503, "Icinga is reloading."); + } + String removedBy (HttpUtility::GetLastParameter(params, "author")); checkable->ClearAcknowledgement(removedBy); @@ -297,6 +310,12 @@ Dictionary::Ptr ApiActions::AddComment(const ConfigObject::Ptr& object, timestamp = HttpUtility::GetLastParameter(params, "expiry"); } + ConfigObjectsSharedLock lock (std::try_to_lock); + + if (!lock) { + return ApiActions::CreateResult(503, "Icinga is reloading."); + } + String commentName = Comment::AddComment(checkable, CommentUser, HttpUtility::GetLastParameter(params, "author"), HttpUtility::GetLastParameter(params, "comment"), false, timestamp); @@ -316,6 +335,12 @@ Dictionary::Ptr ApiActions::AddComment(const ConfigObject::Ptr& object, Dictionary::Ptr ApiActions::RemoveComment(const ConfigObject::Ptr& object, const Dictionary::Ptr& params) { + ConfigObjectsSharedLock lock (std::try_to_lock); + + if (!lock) { + return ApiActions::CreateResult(503, "Icinga is reloading."); + } + auto author (HttpUtility::GetLastParameter(params, "author")); Checkable::Ptr checkable = dynamic_pointer_cast(object); @@ -388,6 +413,12 @@ Dictionary::Ptr ApiActions::ScheduleDowntime(const ConfigObject::Ptr& object, } } + ConfigObjectsSharedLock lock (std::try_to_lock); + + if (!lock) { + return ApiActions::CreateResult(503, "Icinga is reloading."); + } + Downtime::Ptr downtime = Downtime::AddDowntime(checkable, author, comment, startTime, endTime, fixed, triggerName, duration); String downtimeName = downtime->GetName(); @@ -500,6 +531,12 @@ Dictionary::Ptr ApiActions::ScheduleDowntime(const ConfigObject::Ptr& object, Dictionary::Ptr ApiActions::RemoveDowntime(const ConfigObject::Ptr& object, const Dictionary::Ptr& params) { + ConfigObjectsSharedLock lock (std::try_to_lock); + + if (!lock) { + return ApiActions::CreateResult(503, "Icinga is reloading."); + } + auto author (HttpUtility::GetLastParameter(params, "author")); Checkable::Ptr checkable = dynamic_pointer_cast(object); diff --git a/lib/remote/createobjecthandler.cpp b/lib/remote/createobjecthandler.cpp index c01b23641..598eeec3b 100644 --- a/lib/remote/createobjecthandler.cpp +++ b/lib/remote/createobjecthandler.cpp @@ -1,6 +1,7 @@ /* Icinga 2 | (c) 2012 Icinga GmbH | GPLv2+ */ #include "remote/createobjecthandler.hpp" +#include "remote/configobjectslock.hpp" #include "remote/configobjectutility.hpp" #include "remote/httputility.hpp" #include "remote/jsonrpcconnection.hpp" @@ -94,6 +95,13 @@ bool CreateObjectHandler::HandleRequest( if (params) verbose = HttpUtility::GetLastParameter(params, "verbose"); + ConfigObjectsSharedLock lock (std::try_to_lock); + + if (!lock) { + HttpUtility::SendJsonError(response, params, 503, "Icinga is reloading"); + return true; + } + /* Object creation can cause multiple errors and optionally diagnostic information. * We can't use SendJsonError() here. */ diff --git a/lib/remote/deleteobjecthandler.cpp b/lib/remote/deleteobjecthandler.cpp index 2edb0e455..a4fd98d9a 100644 --- a/lib/remote/deleteobjecthandler.cpp +++ b/lib/remote/deleteobjecthandler.cpp @@ -1,6 +1,7 @@ /* Icinga 2 | (c) 2012 Icinga GmbH | GPLv2+ */ #include "remote/deleteobjecthandler.hpp" +#include "remote/configobjectslock.hpp" #include "remote/configobjectutility.hpp" #include "remote/httputility.hpp" #include "remote/filterutility.hpp" @@ -66,6 +67,13 @@ bool DeleteObjectHandler::HandleRequest( bool cascade = HttpUtility::GetLastParameter(params, "cascade"); bool verbose = HttpUtility::GetLastParameter(params, "verbose"); + ConfigObjectsSharedLock lock (std::try_to_lock); + + if (!lock) { + HttpUtility::SendJsonError(response, params, 503, "Icinga is reloading"); + return true; + } + ArrayData results; bool success = true; diff --git a/lib/remote/modifyobjecthandler.cpp b/lib/remote/modifyobjecthandler.cpp index cc008b90c..d9a215f5f 100644 --- a/lib/remote/modifyobjecthandler.cpp +++ b/lib/remote/modifyobjecthandler.cpp @@ -1,6 +1,7 @@ /* Icinga 2 | (c) 2012 Icinga GmbH | GPLv2+ */ #include "remote/modifyobjecthandler.hpp" +#include "remote/configobjectslock.hpp" #include "remote/httputility.hpp" #include "remote/filterutility.hpp" #include "remote/apiaction.hpp" @@ -77,6 +78,13 @@ bool ModifyObjectHandler::HandleRequest( if (params) verbose = HttpUtility::GetLastParameter(params, "verbose"); + ConfigObjectsSharedLock lock (std::try_to_lock); + + if (!lock) { + HttpUtility::SendJsonError(response, params, 503, "Icinga is reloading"); + return true; + } + ArrayData results; for (const ConfigObject::Ptr& obj : objs) {