From 4d80903c47bf8c45130a68883de427d90e97bc97 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Mon, 23 Mar 2020 17:31:59 +0100 Subject: [PATCH 1/2] ApiListener::ConfigUpdateHandler(): block as less as possible refs #7742 --- lib/remote/apilistener-filesync.cpp | 9 +++++++-- lib/remote/apilistener.hpp | 1 + 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/remote/apilistener-filesync.cpp b/lib/remote/apilistener-filesync.cpp index 4371a826c..ae2767e79 100644 --- a/lib/remote/apilistener-filesync.cpp +++ b/lib/remote/apilistener-filesync.cpp @@ -13,6 +13,7 @@ #include "base/utility.hpp" #include #include +#include using namespace icinga; @@ -310,6 +311,12 @@ Value ApiListener::ConfigUpdateHandler(const MessageOrigin::Ptr& origin, const D return Empty; } + std::thread([origin, params]() { HandleConfigUpdate(origin, params); }).detach(); + return Empty; +} + +void ApiListener::HandleConfigUpdate(const MessageOrigin::Ptr& origin, const Dictionary::Ptr& params) +{ /* Only one transaction is allowed, concurrent message handlers need to wait. * This affects two parent endpoints sending the config in the same moment. */ @@ -527,8 +534,6 @@ Value ApiListener::ConfigUpdateHandler(const MessageOrigin::Ptr& origin, const D << "Received configuration updates (" << count << ") from endpoint '" << fromEndpointName << "' do not qualify for production, not triggering reload."; } - - return Empty; } /** diff --git a/lib/remote/apilistener.hpp b/lib/remote/apilistener.hpp index ecbeed415..f6b7f469e 100644 --- a/lib/remote/apilistener.hpp +++ b/lib/remote/apilistener.hpp @@ -83,6 +83,7 @@ public: /* filesync */ static Value ConfigUpdateHandler(const MessageOrigin::Ptr& origin, const Dictionary::Ptr& params); + static void HandleConfigUpdate(const MessageOrigin::Ptr& origin, const Dictionary::Ptr& params); /* configsync */ static void ConfigUpdateObjectHandler(const ConfigObject::Ptr& object, const Value& cookie); From 67b04fae6914829cfb5849f43fb94a9b64084d22 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Mon, 23 Mar 2020 17:33:14 +0100 Subject: [PATCH 2/2] ApiListener::HandleConfigUpdate(): make the whole process mutually exclusive refs #7742 --- lib/remote/apilistener-filesync.cpp | 12 ++++++++---- lib/remote/apilistener.hpp | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/remote/apilistener-filesync.cpp b/lib/remote/apilistener-filesync.cpp index ae2767e79..ab1d5c728 100644 --- a/lib/remote/apilistener-filesync.cpp +++ b/lib/remote/apilistener-filesync.cpp @@ -10,6 +10,7 @@ #include "base/convert.hpp" #include "base/application.hpp" #include "base/exception.hpp" +#include "base/shared.hpp" #include "base/utility.hpp" #include #include @@ -320,7 +321,7 @@ void ApiListener::HandleConfigUpdate(const MessageOrigin::Ptr& origin, const Dic /* Only one transaction is allowed, concurrent message handlers need to wait. * This affects two parent endpoints sending the config in the same moment. */ - boost::mutex::scoped_lock lock(m_ConfigSyncStageLock); + auto lock (Shared::Make(m_ConfigSyncStageLock)); String apiZonesStageDir = GetApiZonesStageDir(); String fromEndpointName = origin->FromClient->GetEndpoint()->GetName(); @@ -528,7 +529,7 @@ void ApiListener::HandleConfigUpdate(const MessageOrigin::Ptr& origin, const Dic Log(LogInformation, "ApiListener") << "Received configuration updates (" << count << ") from endpoint '" << fromEndpointName << "' are different to production, triggering validation and reload."; - AsyncTryActivateZonesStage(relativePaths); + AsyncTryActivateZonesStage(relativePaths, lock); } else { Log(LogInformation, "ApiListener") << "Received configuration updates (" << count << ") from endpoint '" << fromEndpointName @@ -617,7 +618,7 @@ void ApiListener::TryActivateZonesStageCallback(const ProcessResult& pr, * * @param relativePaths Required for later file operations in the callback. Provides the zone name plus path in a list. */ -void ApiListener::AsyncTryActivateZonesStage(const std::vector& relativePaths) +void ApiListener::AsyncTryActivateZonesStage(const std::vector& relativePaths, const Shared::Ptr& lock) { VERIFY(Application::GetArgC() >= 1); @@ -643,7 +644,10 @@ void ApiListener::AsyncTryActivateZonesStage(const std::vector& relative Process::Ptr process = new Process(Process::PrepareCommand(args)); process->SetTimeout(Application::GetReloadTimeout()); - process->Run(std::bind(&TryActivateZonesStageCallback, _1, relativePaths)); + + process->Run([relativePaths, lock](const ProcessResult& pr) { + TryActivateZonesStageCallback(pr, relativePaths); + }); } /** diff --git a/lib/remote/apilistener.hpp b/lib/remote/apilistener.hpp index f6b7f469e..e61df2c47 100644 --- a/lib/remote/apilistener.hpp +++ b/lib/remote/apilistener.hpp @@ -192,7 +192,7 @@ private: static void TryActivateZonesStageCallback(const ProcessResult& pr, const std::vector& relativePaths); - static void AsyncTryActivateZonesStage(const std::vector& relativePaths); + static void AsyncTryActivateZonesStage(const std::vector& relativePaths, const Shared::Ptr& lock); static String GetChecksum(const String& content); static bool CheckConfigChange(const ConfigDirInformation& oldConfig, const ConfigDirInformation& newConfig);