From 704aabcb63c81ca15d858be68c964d8c3f50300f Mon Sep 17 00:00:00 2001 From: Michael Friedrich Date: Wed, 8 May 2019 16:06:46 +0200 Subject: [PATCH] Avoid dead-lock with config packages and active stages --- lib/remote/configobjectutility.cpp | 3 ++- lib/remote/configpackageshandler.cpp | 6 ++++-- lib/remote/configpackageutility.cpp | 15 +++++++++++---- lib/remote/configpackageutility.hpp | 3 ++- lib/remote/configstageshandler.cpp | 3 ++- 5 files changed, 21 insertions(+), 9 deletions(-) diff --git a/lib/remote/configobjectutility.cpp b/lib/remote/configobjectutility.cpp index 77714e4a9..503cdbb45 100644 --- a/lib/remote/configobjectutility.cpp +++ b/lib/remote/configobjectutility.cpp @@ -85,7 +85,8 @@ bool ConfigObjectUtility::CreateObject(const Type::Ptr& type, const String& full const String& config, const Array::Ptr& errors, const Array::Ptr& diagnosticInformation) { { - boost::mutex::scoped_lock lock(ConfigPackageUtility::GetStaticMutex()); + boost::mutex::scoped_lock lock(ConfigPackageUtility::GetStaticPackageMutex()); + if (!ConfigPackageUtility::PackageExists("_api")) { ConfigPackageUtility::CreatePackage("_api"); diff --git a/lib/remote/configpackageshandler.cpp b/lib/remote/configpackageshandler.cpp index d9cd9ec9b..fd60661f7 100644 --- a/lib/remote/configpackageshandler.cpp +++ b/lib/remote/configpackageshandler.cpp @@ -63,7 +63,8 @@ void ConfigPackagesHandler::HandleGet( ArrayData results; { - boost::mutex::scoped_lock lock(ConfigPackageUtility::GetStaticMutex()); + boost::mutex::scoped_lock lock(ConfigPackageUtility::GetStaticPackageMutex()); + for (const String& package : packages) { results.emplace_back(new Dictionary({ { "name", package }, @@ -104,7 +105,8 @@ void ConfigPackagesHandler::HandlePost( } try { - boost::mutex::scoped_lock lock(ConfigPackageUtility::GetStaticMutex()); + boost::mutex::scoped_lock lock(ConfigPackageUtility::GetStaticPackageMutex()); + ConfigPackageUtility::CreatePackage(packageName); } catch (const std::exception& ex) { HttpUtility::SendJsonError(response, params, 500, "Could not create package '" + packageName + "'.", diff --git a/lib/remote/configpackageutility.cpp b/lib/remote/configpackageutility.cpp index b56bbb9fc..d0bf90061 100644 --- a/lib/remote/configpackageutility.cpp +++ b/lib/remote/configpackageutility.cpp @@ -186,7 +186,8 @@ void ConfigPackageUtility::TryActivateStageCallback(const ProcessResult& pr, con /* validation went fine, activate stage and reload */ if (pr.ExitStatus == 0) { { - boost::mutex::scoped_lock lock(GetStaticMutex()); + boost::mutex::scoped_lock lock(GetStaticPackageMutex()); + ActivateStage(packageName, stageName); } @@ -251,7 +252,7 @@ std::vector ConfigPackageUtility::GetStages(const String& packageName) String ConfigPackageUtility::GetActiveStageFromFile(const String& packageName) { /* Lock the transaction, reading this only happens on startup or when something really is broken. */ - boost::mutex::scoped_lock lock(GetStaticMutex()); + boost::mutex::scoped_lock lock(GetStaticActiveStageMutex()); String path = GetPackageDir() + "/" + packageName + "/active-stage"; @@ -271,7 +272,7 @@ String ConfigPackageUtility::GetActiveStageFromFile(const String& packageName) void ConfigPackageUtility::SetActiveStageToFile(const String& packageName, const String& stageName) { - boost::mutex::scoped_lock lock(GetStaticMutex()); + boost::mutex::scoped_lock lock(GetStaticActiveStageMutex()); String activeStagePath = GetPackageDir() + "/" + packageName + "/active-stage"; @@ -380,7 +381,13 @@ bool ConfigPackageUtility::ValidateName(const String& name) return (!boost::regex_search(name.GetData(), what, expr)); } -boost::mutex& ConfigPackageUtility::GetStaticMutex() +boost::mutex& ConfigPackageUtility::GetStaticPackageMutex() +{ + static boost::mutex mutex; + return mutex; +} + +boost::mutex& ConfigPackageUtility::GetStaticActiveStageMutex() { static boost::mutex mutex; return mutex; diff --git a/lib/remote/configpackageutility.hpp b/lib/remote/configpackageutility.hpp index 11b2b9977..9f105794c 100644 --- a/lib/remote/configpackageutility.hpp +++ b/lib/remote/configpackageutility.hpp @@ -44,7 +44,8 @@ public: static bool ContainsDotDot(const String& path); static bool ValidateName(const String& name); - static boost::mutex& GetStaticMutex(); + static boost::mutex& GetStaticPackageMutex(); + static boost::mutex& GetStaticActiveStageMutex(); private: static void CollectDirNames(const String& path, std::vector& dirs); diff --git a/lib/remote/configstageshandler.cpp b/lib/remote/configstageshandler.cpp index a3c570a2f..2730ac37b 100644 --- a/lib/remote/configstageshandler.cpp +++ b/lib/remote/configstageshandler.cpp @@ -120,7 +120,8 @@ void ConfigStagesHandler::HandlePost( if (!files) BOOST_THROW_EXCEPTION(std::invalid_argument("Parameter 'files' must be specified.")); - boost::mutex::scoped_lock lock(ConfigPackageUtility::GetStaticMutex()); + boost::mutex::scoped_lock lock(ConfigPackageUtility::GetStaticPackageMutex()); + stageName = ConfigPackageUtility::CreateStage(packageName, files); /* validate the config. on success, activate stage and reload */