Log lock acquisitions/releases

librepomgr/CMakeLists.txt

@@ -6,8 +6,10 @@ set(HEADER_FILES
     serversetup.h
     helper.h
     json.h
+    logcontext.h
     logging.h
     multisession.h
+    namedlockable.h
     authentication.h
     webapi/server.h
     webapi/session.h
@@ -30,6 +32,7 @@ set(SRC_FILES
     json.cpp
     errorhandling.cpp
     serversetup.cpp
+    namedlockable.cpp
     authentication.cpp
     webapi/server.cpp
     webapi/session.cpp

librepomgr/buildactions/buildaction.h

@@ -7,6 +7,9 @@
 
 #include "../webapi/routes.h"
 
+#include "../logcontext.h"
+#include "../namedlockable.h"
+
 #include "../../libpkg/data/config.h"
 #include "../../libpkg/data/lockable.h"
 
@@ -35,22 +38,6 @@ class BuildActionsTests;
 
 namespace LibRepoMgr {
 
-struct LogContext {
-    explicit LogContext(BuildAction *buildAction = nullptr);
-    LogContext &operator=(const LogContext &) = delete;
-    template <typename... Args> LogContext &operator()(CppUtilities::EscapeCodes::Phrases phrase, Args &&...args);
-    template <typename... Args> LogContext &operator()(Args &&...args);
-    template <typename... Args> LogContext &operator()(std::string &&msg);
-
-private:
-    BuildAction *const m_buildAction;
-};
-
-inline LogContext::LogContext(BuildAction *buildAction)
-    : m_buildAction(buildAction)
-{
-}
-
 struct ServiceSetup;
 
 namespace WebAPI {
@@ -60,7 +47,7 @@ class Session;
 
 struct InternalBuildAction;
 
-using AssociatedLocks = std::vector<std::variant<std::shared_lock<std::shared_mutex>, std::unique_lock<std::shared_mutex>>>;
+using AssociatedLocks = std::vector<std::variant<SharedNamedLock, UniqueNamedLock>>;
 
 struct LIBREPOMGR_EXPORT PackageBuildData : public ReflectiveRapidJSON::JsonSerializable<PackageBuildData>,
                                             public ReflectiveRapidJSON::BinarySerializable<PackageBuildData> {
@@ -362,3 +349,6 @@ struct LIBREPOMGR_EXPORT BuildActionBasicInfo : public ReflectiveRapidJSON::Json
 } // namespace LibRepoMgr
 
 #endif // LIBREPOMGR_BUILD_ACTION_H
+
+// avoid making LogContext available without also defining overloads for operator() which would possibly lead to linker errors
+#include "../logging.h"

librepomgr/buildactions/conductbuild.cpp

@@ -934,7 +934,7 @@ InvocationResult ConductBuild::invokeMakechrootpkg(
 
     // lock the chroot directory to prevent other build tasks from using it
     m_buildAction->log()(Phrases::InfoMessage, "Building ", packageName, '\n');
-    auto chrootLock = m_setup.locks.acquireToWrite(buildRoot);
+    auto chrootLock = m_setup.locks.acquireToWrite(m_buildAction->log(), std::string(buildRoot));
 
     // copy config files into chroot directory
     try {
@@ -1117,7 +1117,7 @@ void ConductBuild::addPackageToRepo(
     auto processSession = m_buildAction->makeBuildProcess("repo-add for " + packageName, packageProgress.buildDirectory + "/repo-add.log",
         std::bind(&ConductBuild::handleRepoAddErrorsAndMakeNextPackage, this, makepkgchrootSession, std::ref(packageName), std::ref(packageProgress),
             std::placeholders::_1, std::placeholders::_2));
-    processSession->locks().emplace_back(m_setup.locks.acquireToWrite(
+    processSession->locks().emplace_back(m_setup.locks.acquireToWrite(m_buildAction->log(),
         ServiceSetup::Locks::forDatabase(needsStaging ? m_buildPreparation.targetDb : m_buildPreparation.stagingDb, m_buildPreparation.targetArch)));
     processSession->launch(boost::process::start_dir(repoPath), m_repoAddPath, dbFilePath, binaryPackageNames);
     m_buildAction->log()(Phrases::InfoMessage, "Adding ", packageName, " to repo\n", ps(Phrases::SubMessage), "repo path: ", repoPath, '\n',

librepomgr/buildactions/customcommand.cpp

@@ -80,15 +80,16 @@ void CustomCommand::run()
     const auto sharedLockNames = splitStringSimple<std::set<std::string>>(findSetting(sharedLocksSetting), ",");
     const auto exclusiveLockNames = splitStringSimple<std::set<std::string>>(findSetting(exclusiveLocksSetting), ",");
     auto &locks = process->locks();
+    auto &log = m_buildAction->log();
     locks.reserve(sharedLockNames.size() + exclusiveLockNames.size());
     for (const auto &lockName : sharedLockNames) {
         if (!lockName.empty()) {
-            locks.emplace_back(m_setup.locks.acquireToRead(lockName));
+            locks.emplace_back(m_setup.locks.acquireToRead(log, std::string(lockName)));
         }
     }
     for (const auto &lockName : exclusiveLockNames) {
         if (!lockName.empty()) {
-            locks.emplace_back(m_setup.locks.acquireToWrite(lockName));
+            locks.emplace_back(m_setup.locks.acquireToWrite(log, std::string(lockName)));
         }
     }
 

librepomgr/buildactions/reloaddatabase.cpp

@@ -74,11 +74,11 @@ void ReloadDatabase::run()
                 m_buildAction->appendOutput(
                     Phrases::InfoMessage, "Loading database \"", dbName, '@', dbArch, "\" from local file \"", dbPath, "\"\n");
                 try {
-                    auto dbFileLock = m_setup.locks.acquireToRead({ ServiceSetup::Locks::forDatabase(dbName, dbArch) });
+                    auto dbFileLock = m_setup.locks.acquireToRead(m_buildAction->log(), ServiceSetup::Locks::forDatabase(dbName, dbArch));
                     const auto lastModified = LibPkg::lastModified(dbPath);
                     auto dbFile = LibPkg::extractFiles(dbPath, &LibPkg::Database::isFileRelevant);
                     auto packages = LibPkg::Package::fromDatabaseFile(move(dbFile));
-                    dbFileLock.unlock();
+                    dbFileLock.lock().unlock();
                     const auto configLock = m_setup.config.lockToWrite();
                     auto *const db = m_setup.config.findDatabase(dbName, dbArch);
                     if (!db) {

librepomgr/buildactions/repomanagement.cpp

@@ -156,7 +156,7 @@ void RemovePackages::run()
     // remove package from database file
     auto repoRemoveProcess = m_buildAction->makeBuildProcess("repo-remove", m_workingDirectory + "/repo-remove.log",
         std::bind(&RemovePackages::handleRepoRemoveResult, this, std::placeholders::_1, std::placeholders::_2));
-    repoRemoveProcess->locks().emplace_back(m_setup.locks.acquireToWrite(m_destinationDatabaseLockName));
+    repoRemoveProcess->locks().emplace_back(m_setup.locks.acquireToWrite(m_buildAction->log(), std::move(m_destinationDatabaseLockName)));
     repoRemoveProcess->launch(
         boost::process::start_dir(m_destinationRepoDirectory), m_repoRemovePath, m_destinationDatabaseFile, m_result.processedPackages);
     m_buildAction->log()(Phrases::InfoMessage, "Invoking repo-remove within \"", m_destinationRepoDirectory, "\" for \"", m_destinationDatabaseFile,
@@ -273,13 +273,13 @@ void MovePackages::run()
     // add packages to database file of destination repo
     auto repoAddProcess = m_buildAction->makeBuildProcess("repo-add", m_workingDirectory + "/repo-add.log",
         std::bind(&MovePackages::handleRepoAddResult, this, processSession, std::placeholders::_1, std::placeholders::_2));
-    repoAddProcess->locks().emplace_back(m_setup.locks.acquireToWrite(m_destinationDatabaseLockName));
+    repoAddProcess->locks().emplace_back(m_setup.locks.acquireToWrite(m_buildAction->log(), std::move(m_destinationDatabaseLockName)));
     repoAddProcess->launch(boost::process::start_dir(m_destinationRepoDirectory), m_repoAddPath, m_destinationDatabaseFile, m_fileNames);
 
     // remove package from database file of source repo
     auto repoRemoveProcess = m_buildAction->makeBuildProcess("repo-remove", m_workingDirectory + "/repo-remove.log",
         std::bind(&MovePackages::handleRepoRemoveResult, this, processSession, std::placeholders::_1, std::placeholders::_2));
-    repoRemoveProcess->locks().emplace_back(m_setup.locks.acquireToWrite(m_sourceDatabaseLockName));
+    repoRemoveProcess->locks().emplace_back(m_setup.locks.acquireToWrite(m_buildAction->log(), std::move(m_sourceDatabaseLockName)));
     repoRemoveProcess->launch(boost::process::start_dir(m_sourceRepoDirectory), m_repoRemovePath, m_sourceDatabaseFile, m_result.processedPackages);
 
     m_buildAction->log()(ps(Phrases::InfoMessage), "Invoking repo-add within \"", m_destinationRepoDirectory, "\" for \"", m_destinationDatabaseFile,

librepomgr/logcontext.h

@@ -0,0 +1,33 @@
+#ifndef LIBREPOMGR_LOGCONTEXT_H
+#define LIBREPOMGR_LOGCONTEXT_H
+
+// Do NOT include this header directly, include "loggin.h" instead. This header only exists to resolve the
+// cyclic dependency between LogContext and BuildAction but lacks definitions of operator().
+
+#include "./global.h"
+
+#include <c++utilities/io/ansiescapecodes.h>
+
+namespace LibRepoMgr {
+
+struct BuildAction;
+
+struct LIBREPOMGR_EXPORT LogContext {
+    explicit LogContext(BuildAction *buildAction = nullptr);
+    LogContext &operator=(const LogContext &) = delete;
+    template <typename... Args> LogContext &operator()(CppUtilities::EscapeCodes::Phrases phrase, Args &&...args);
+    template <typename... Args> LogContext &operator()(Args &&...args);
+    template <typename... Args> LogContext &operator()(std::string &&msg);
+
+private:
+    BuildAction *const m_buildAction;
+};
+
+inline LogContext::LogContext(BuildAction *buildAction)
+    : m_buildAction(buildAction)
+{
+}
+
+} // namespace LibRepoMgr
+
+#endif // LIBREPOMGR_LOGCONTEXT_H

librepomgr/logging.h

@@ -1,6 +1,8 @@
 #ifndef LIBREPOMGR_LOGGING_H
 #define LIBREPOMGR_LOGGING_H
 
+#include "./logcontext.h"
+
 #include "./buildactions/buildaction.h"
 
 namespace LibRepoMgr {
@@ -10,7 +12,7 @@ inline auto ps(CppUtilities::EscapeCodes::Phrases phrase)
     return CppUtilities::EscapeCodes::formattedPhraseString(phrase);
 }
 
-template <typename... Args> LogContext &LogContext::operator()(std::string &&msg)
+template <typename... Args> LIBREPOMGR_EXPORT LogContext &LogContext::operator()(std::string &&msg)
 {
     std::cerr << msg;
     if (m_buildAction) {
@@ -19,12 +21,12 @@ template <typename... Args> LogContext &LogContext::operator()(std::string &&msg
     return *this;
 }
 
-template <typename... Args> LogContext &LogContext::operator()(CppUtilities::EscapeCodes::Phrases phrase, Args &&...args)
+template <typename... Args> inline LogContext &LogContext::operator()(CppUtilities::EscapeCodes::Phrases phrase, Args &&...args)
 {
     return (*this)(CppUtilities::argsToString(CppUtilities::EscapeCodes::formattedPhraseString(phrase), std::forward<Args>(args)...));
 }
 
-template <typename... Args> LogContext &LogContext::operator()(Args &&...args)
+template <typename... Args> inline LogContext &LogContext::operator()(Args &&...args)
 {
     return (*this)(CppUtilities::argsToString(
         CppUtilities::EscapeCodes::formattedPhraseString(CppUtilities::EscapeCodes::Phrases::InfoMessage), std::forward<Args>(args)...));

librepomgr/namedlockable.cpp

@@ -0,0 +1,9 @@
+#include "./namedlockable.h"
+#include "./logging.h"
+
+namespace LibRepoMgr {
+
+template struct NamedLock<std::shared_lock<std::shared_mutex>>;
+template struct NamedLock<std::unique_lock<std::shared_mutex>>;
+
+} // namespace LibRepoMgr

librepomgr/namedlockable.h

@@ -0,0 +1,105 @@
+#ifndef LIBREPOMGR_NAMED_LOCKABLE_H
+#define LIBREPOMGR_NAMED_LOCKABLE_H
+
+#include <mutex>
+#include <shared_mutex>
+
+#include "./global.h"
+
+namespace LibRepoMgr {
+
+struct LogContext;
+
+template <typename UnderlyingLockType> struct NamedLock {
+    template <typename... Args> NamedLock(LogContext &log, std::string &&name, Args &&...args);
+    NamedLock(NamedLock &&) = default;
+    ~NamedLock();
+
+    const std::string &name() const
+    {
+        return m_name;
+    };
+    UnderlyingLockType &lock()
+    {
+        return m_lock;
+    };
+
+private:
+    LogContext &m_log;
+    std::string m_name;
+    UnderlyingLockType m_lock;
+};
+
+constexpr std::string_view lockName(std::shared_lock<std::shared_mutex> &)
+{
+    return "shared";
+}
+constexpr std::string_view lockName(std::unique_lock<std::shared_mutex> &)
+{
+    return "exclusive";
+}
+
+template <typename UnderlyingLockType>
+template <typename... Args>
+inline NamedLock<UnderlyingLockType>::NamedLock(LogContext &log, std::string &&name, Args &&...args)
+    : m_log(log)
+    , m_name(std::move(name))
+{
+    m_log("Acquiring ", lockName(m_lock), " lock \"", m_name, "\"\n");
+    m_lock = UnderlyingLockType(std::forward<Args>(args)...);
+}
+
+template <typename UnderlyingLockType> inline NamedLock<UnderlyingLockType>::~NamedLock()
+{
+    if (m_lock) {
+        m_lock.unlock();
+        m_log("Released ", lockName(m_lock), " lock \"", m_name, "\"\n");
+    }
+}
+
+using SharedNamedLock = NamedLock<std::shared_lock<std::shared_mutex>>;
+using UniqueNamedLock = NamedLock<std::unique_lock<std::shared_mutex>>;
+
+extern template struct LIBREPOMGR_EXPORT NamedLock<std::shared_lock<std::shared_mutex>>;
+extern template struct LIBREPOMGR_EXPORT NamedLock<std::unique_lock<std::shared_mutex>>;
+
+struct NamedLockable {
+    [[nodiscard]] SharedNamedLock lockToRead(LogContext &log, std::string &&name) const;
+    [[nodiscard]] UniqueNamedLock lockToWrite(LogContext &log, std::string &&name);
+    [[nodiscard]] SharedNamedLock tryLockToRead(LogContext &log, std::string &&name) const;
+    [[nodiscard]] UniqueNamedLock tryLockToWrite(LogContext &log, std::string &&name);
+    [[nodiscard]] UniqueNamedLock lockToWrite(LogContext &log, std::string &&name, SharedNamedLock &readLock);
+
+private:
+    mutable std::shared_mutex m_mutex;
+};
+
+inline SharedNamedLock NamedLockable::lockToRead(LogContext &log, std::string &&name) const
+{
+    return SharedNamedLock(log, std::move(name), m_mutex);
+}
+
+inline UniqueNamedLock NamedLockable::lockToWrite(LogContext &log, std::string &&name)
+{
+    return UniqueNamedLock(log, std::move(name), m_mutex);
+}
+
+inline SharedNamedLock NamedLockable::tryLockToRead(LogContext &log, std::string &&name) const
+{
+    return SharedNamedLock(log, std::move(name), m_mutex, std::try_to_lock);
+}
+
+inline UniqueNamedLock NamedLockable::tryLockToWrite(LogContext &log, std::string &&name)
+{
+    return UniqueNamedLock(log, std::move(name), m_mutex, std::try_to_lock);
+}
+
+inline UniqueNamedLock NamedLockable::lockToWrite(LogContext &log, std::string &&name, SharedNamedLock &readLock)
+{
+    readLock.lock().unlock();
+    return UniqueNamedLock(log, std::move(name), m_mutex);
+}
+
+} // namespace LibRepoMgr
+
+#endif // LIBREPOMGR_NAMED_LOCKABLE_H

librepomgr/serversetup.cpp

@@ -654,10 +654,11 @@ void ServiceSetup::run()
 
 void ServiceSetup::Locks::clear()
 {
+    auto log = LogContext();
     const auto lock = std::lock_guard(m_mutex);
     for (auto i = m_locksByName.begin(), end = m_locksByName.end(); i != end; ++i) {
-        if (auto lock2 = i->second.tryLockToWrite()) { // check whether nobody holds the lock anymore
-            lock2.unlock(); // ~shared_mutex(): The behavior is undefined if the mutex is owned by any thread [...].
+        if (auto lock2 = i->second.tryLockToWrite(log, std::string(i->first)); lock2.lock()) { // check whether nobody holds the lock anymore
+            lock2.lock().unlock(); // ~shared_mutex(): The behavior is undefined if the mutex is owned by any thread [...].
             m_locksByName.erase(i); // we can be sure no other thead aquires i->second in the meantime because we're holding m_mutex
         }
     }

librepomgr/serversetup.h

@@ -4,6 +4,7 @@
 #include "./authentication.h"
 #include "./buildactions/buildaction.h"
 #include "./buildactions/buildactiontemplate.h"
+#include "./namedlockable.h"
 
 #include "../libpkg/data/config.h"
 #include "../libpkg/data/lockable.h"
@@ -121,16 +122,15 @@ struct LIBREPOMGR_EXPORT ServiceSetup : public LibPkg::Lockable {
     } auth;
 
     struct LIBREPOMGR_EXPORT Locks {
-        [[nodiscard]] std::shared_lock<std::shared_mutex> acquireToRead(const std::string &lockName);
-        [[nodiscard]] std::unique_lock<std::shared_mutex> acquireToWrite(const std::string &lockName);
-        [[nodiscard]] std::unique_lock<std::shared_mutex> acquireToWrite(std::shared_lock<std::shared_mutex> &readLock, const std::string &lockName);
+        [[nodiscard]] SharedNamedLock acquireToRead(LogContext &log, std::string &&lockName);
+        [[nodiscard]] UniqueNamedLock acquireToWrite(LogContext &log, std::string &&lockName);
         void clear();
         static std::string forDatabase(std::string_view dbName, std::string_view dbArch);
         static std::string forDatabase(const LibPkg::Database &db);
 
     private:
         std::mutex m_mutex;
-        std::unordered_map<std::string, LibPkg::Lockable> m_locksByName;
+        std::unordered_map<std::string, NamedLockable> m_locksByName;
     } locks;
 
     void loadConfigFiles(bool restoreStateAndDiscardDatabases);
@@ -149,23 +149,16 @@ inline std::shared_ptr<BuildAction> ServiceSetup::BuildSetup::getBuildAction(Bui
     return id < actions.size() ? actions[id] : nullptr;
 }
 
-inline std::shared_lock<std::shared_mutex> ServiceSetup::Locks::acquireToRead(const std::string &lockName)
+inline SharedNamedLock ServiceSetup::Locks::acquireToRead(LogContext &log, std::string &&lockName)
 {
     const auto lock = std::lock_guard(m_mutex);
-    return m_locksByName[lockName].lockToRead();
+    return m_locksByName[lockName].lockToRead(log, std::move(lockName));
 }
 
-inline std::unique_lock<std::shared_mutex> ServiceSetup::Locks::acquireToWrite(const std::string &lockName)
+inline UniqueNamedLock ServiceSetup::Locks::acquireToWrite(LogContext &log, std::string &&lockName)
 {
     const auto lock = std::lock_guard(m_mutex);
-    return m_locksByName[lockName].lockToWrite();
-}
-
-inline std::unique_lock<std::shared_mutex> ServiceSetup::Locks::acquireToWrite(
-    std::shared_lock<std::shared_mutex> &readLock, const std::string &lockName)
-{
-    readLock.unlock();
-    return acquireToWrite(lockName);
+    return m_locksByName[lockName].lockToWrite(log, std::move(lockName));
 }
 
 struct LIBREPOMGR_EXPORT ServiceStatus : public ReflectiveRapidJSON::JsonSerializable<ServiceStatus> {