From f3ec908bcc4de61969615de9f6c1faf6c21271aa Mon Sep 17 00:00:00 2001
From: Martchus <martchus@gmx.net>
Date: Sun, 25 Jul 2021 00:39:49 +0200
Subject: [PATCH] Hold lock for named lock table not while acquiring named lock

Otherwise all other attempts to acquire named locks are blocked. It should
be ok because references are not invalidated when inserting/accessing
elements of an `std::unique_map`. When cleaning locks up elements are
erased, though. Hence an additional cleanup lock is required to prevent
acquiring a named lock which has already been cleaned up.
---
 librepomgr/serversetup.cpp |  2 +-
 librepomgr/serversetup.h   | 24 ++++++++++++++++--------
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/librepomgr/serversetup.cpp b/librepomgr/serversetup.cpp
index af4ed93..fe274df 100644
--- a/librepomgr/serversetup.cpp
+++ b/librepomgr/serversetup.cpp
@@ -660,7 +660,7 @@ void ServiceSetup::run()
 void ServiceSetup::Locks::clear()
 {
     auto log = LogContext();
-    const auto lock = std::lock_guard(m_mutex);
+    const auto lock = std::unique_lock(m_cleanupMutex);
     for (auto i = m_locksByName.begin(), end = m_locksByName.end(); i != end;) {
         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 [...].
diff --git a/librepomgr/serversetup.h b/librepomgr/serversetup.h
index 86a306b..51ac318 100644
--- a/librepomgr/serversetup.h
+++ b/librepomgr/serversetup.h
@@ -127,15 +127,17 @@ struct LIBREPOMGR_EXPORT ServiceSetup : public LibPkg::Lockable {
     struct LIBREPOMGR_EXPORT Locks {
         using LockTable = std::unordered_map<std::string, GlobalLockable>;
 
+        [[nodiscard]] GlobalLockable &namedLock(const std::string &lockName);
         [[nodiscard]] SharedLoggingLock acquireToRead(LogContext &log, std::string &&lockName);
         [[nodiscard]] UniqueLoggingLock acquireToWrite(LogContext &log, std::string &&lockName);
-        [[nodiscard]] std::pair<LockTable *, std::unique_lock<std::mutex>> acquireLockTable();
+        [[nodiscard]] std::pair<LockTable *, std::unique_lock<std::shared_mutex>> acquireLockTable();
         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::mutex m_accessMutex;
+        std::shared_mutex m_cleanupMutex;
         LockTable m_locksByName;
     } locks;
 
@@ -155,21 +157,27 @@ inline std::shared_ptr<BuildAction> ServiceSetup::BuildSetup::getBuildAction(Bui
     return id < actions.size() ? actions[id] : nullptr;
 }
 
+inline GlobalLockable &ServiceSetup::Locks::namedLock(const std::string &lockName)
+{
+    const auto locktableLock = std::unique_lock(m_accessMutex);
+    return m_locksByName[lockName];
+}
+
 inline SharedLoggingLock ServiceSetup::Locks::acquireToRead(LogContext &log, std::string &&lockName)
 {
-    const auto lock = std::lock_guard(m_mutex);
-    return m_locksByName[lockName].lockToRead(log, std::move(lockName));
+    const auto locktableLock = std::shared_lock(m_cleanupMutex);
+    return namedLock(lockName).lockToRead(log, std::move(lockName));
 }
 
 inline UniqueLoggingLock ServiceSetup::Locks::acquireToWrite(LogContext &log, std::string &&lockName)
 {
-    const auto lock = std::lock_guard(m_mutex);
-    return m_locksByName[lockName].lockToWrite(log, std::move(lockName));
+    const auto locktableLock = std::shared_lock(m_cleanupMutex);
+    return namedLock(lockName).lockToWrite(log, std::move(lockName));
 }
 
-inline std::pair<ServiceSetup::Locks::LockTable *, std::unique_lock<std::mutex>> ServiceSetup::Locks::acquireLockTable()
+inline std::pair<ServiceSetup::Locks::LockTable *, std::unique_lock<std::shared_mutex>> ServiceSetup::Locks::acquireLockTable()
 {
-    return std::make_pair(&m_locksByName, std::unique_lock(m_mutex));
+    return std::make_pair(&m_locksByName, std::unique_lock(m_cleanupMutex));
 }
 
 struct LIBREPOMGR_EXPORT ServiceStatus : public ReflectiveRapidJSON::JsonSerializable<ServiceStatus> {