diff --git a/librepomgr/CMakeLists.txt b/librepomgr/CMakeLists.txt index 9a56efa..7909dd9 100644 --- a/librepomgr/CMakeLists.txt +++ b/librepomgr/CMakeLists.txt @@ -9,7 +9,7 @@ set(HEADER_FILES logcontext.h logging.h multisession.h - namedlockable.h + globallock.h authentication.h webapi/server.h webapi/session.h @@ -32,7 +32,7 @@ set(SRC_FILES json.cpp errorhandling.cpp serversetup.cpp - namedlockable.cpp + globallock.cpp authentication.cpp webapi/server.cpp webapi/session.cpp @@ -58,7 +58,7 @@ set(SRC_FILES buildactions/preparebuild.cpp buildactions/conductbuild.cpp) set(TEST_HEADER_FILES tests/parser_helper.h) -set(TEST_SRC_FILES tests/cppunit.cpp tests/buildactions.cpp tests/webapi.cpp tests/parser_helper.cpp) +set(TEST_SRC_FILES tests/cppunit.cpp tests/buildactions.cpp tests/utils.cpp tests/webapi.cpp tests/parser_helper.cpp) # meta data set(META_PROJECT_NAME librepomgr) diff --git a/librepomgr/buildactions/buildaction.h b/librepomgr/buildactions/buildaction.h index 38bcef6..57caad3 100644 --- a/librepomgr/buildactions/buildaction.h +++ b/librepomgr/buildactions/buildaction.h @@ -7,8 +7,8 @@ #include "../webapi/routes.h" +#include "../globallock.h" #include "../logcontext.h" -#include "../namedlockable.h" #include "../../libpkg/data/config.h" #include "../../libpkg/data/lockable.h" @@ -47,7 +47,7 @@ class Session; struct InternalBuildAction; -using AssociatedLocks = std::vector>; +using AssociatedLocks = std::vector>; struct LIBREPOMGR_EXPORT PackageBuildData : public ReflectiveRapidJSON::JsonSerializable, public ReflectiveRapidJSON::BinarySerializable { diff --git a/librepomgr/globallock.cpp b/librepomgr/globallock.cpp new file mode 100644 index 0000000..0becdee --- /dev/null +++ b/librepomgr/globallock.cpp @@ -0,0 +1,9 @@ +#include "./globallock.h" +#include "./logging.h" + +namespace LibRepoMgr { + +template struct LoggingLock>; +template struct LoggingLock>; + +} // namespace LibRepoMgr diff --git a/librepomgr/globallock.h b/librepomgr/globallock.h new file mode 100644 index 0000000..9ab4ef1 --- /dev/null +++ b/librepomgr/globallock.h @@ -0,0 +1,178 @@ +#ifndef LIBREPOMGR_GLOBAL_LOCK_H +#define LIBREPOMGR_GLOBAL_LOCK_H + +#include +#include +#include + +#include "./global.h" + +namespace LibRepoMgr { + +struct LogContext; + +/// \brief A shared mutex where ownership is not tied to a thread (similar to a binary semaphore in that regard). +struct GlobalSharedMutex { + void lock(); + bool try_lock(); + void unlock(); + + void lock_shared(); + bool try_lock_shared(); + void unlock_shared(); + +private: + std::mutex m_mutex; + std::condition_variable m_cv; + std::uint32_t m_sharedOwners = 0; + bool m_exclusivelyOwned = false; +}; + +inline void GlobalSharedMutex::lock() +{ + auto lock = std::unique_lock(m_mutex); + if (m_sharedOwners || m_exclusivelyOwned) { + m_cv.wait(lock); + } + m_exclusivelyOwned = true; +} + +inline bool GlobalSharedMutex::try_lock() +{ + auto lock = std::unique_lock(m_mutex); + if (m_sharedOwners || m_exclusivelyOwned) { + return false; + } else { + return m_exclusivelyOwned = true; + } +} + +inline void GlobalSharedMutex::unlock() +{ + m_exclusivelyOwned = false; + m_cv.notify_one(); +} + +inline void GlobalSharedMutex::lock_shared() +{ + auto lock = std::unique_lock(m_mutex); + if (m_exclusivelyOwned) { + m_cv.wait(lock); + } + ++m_sharedOwners; +} + +inline bool GlobalSharedMutex::try_lock_shared() +{ + auto lock = std::unique_lock(m_mutex); + if (m_exclusivelyOwned) { + return false; + } else { + return ++m_sharedOwners; + } +} + +inline void GlobalSharedMutex::unlock_shared() +{ + auto lock = std::unique_lock(m_mutex); + if (!--m_sharedOwners) { + lock.unlock(); + m_cv.notify_one(); + } +} + +/// \brief A wrapper around a standard lock which logs acquisition/release. +template struct LoggingLock { + template LoggingLock(LogContext &log, std::string &&name, Args &&...args); + LoggingLock(LoggingLock &&) = default; + ~LoggingLock(); + + 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 &) +{ + return "shared"; +} +constexpr std::string_view lockName(std::unique_lock &) +{ + return "exclusive"; +} + +template +template +inline LoggingLock::LoggingLock(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)...); +} + +template inline LoggingLock::~LoggingLock() +{ + if (m_lock) { + m_lock.unlock(); + m_log("Released ", lockName(m_lock), " lock \"", m_name, "\"\n"); + } +} + +using SharedLoggingLock = LoggingLock>; +using UniqueLoggingLock = LoggingLock>; + +extern template struct LIBREPOMGR_EXPORT LoggingLock>; +extern template struct LIBREPOMGR_EXPORT LoggingLock>; + +/// \brief Same as LibPkg::Lockable but using GlobalSharedMutex. +struct GlobalLockable { + [[nodiscard]] SharedLoggingLock lockToRead(LogContext &log, std::string &&name) const; + [[nodiscard]] UniqueLoggingLock lockToWrite(LogContext &log, std::string &&name); + [[nodiscard]] SharedLoggingLock tryLockToRead(LogContext &log, std::string &&name) const; + [[nodiscard]] UniqueLoggingLock tryLockToWrite(LogContext &log, std::string &&name); + [[nodiscard]] UniqueLoggingLock lockToWrite(LogContext &log, std::string &&name, SharedLoggingLock &readLock); + +private: + mutable GlobalSharedMutex m_mutex; +}; + +inline SharedLoggingLock GlobalLockable::lockToRead(LogContext &log, std::string &&name) const +{ + return SharedLoggingLock(log, std::move(name), m_mutex); +} + +inline UniqueLoggingLock GlobalLockable::lockToWrite(LogContext &log, std::string &&name) +{ + return UniqueLoggingLock(log, std::move(name), m_mutex); +} + +inline SharedLoggingLock GlobalLockable::tryLockToRead(LogContext &log, std::string &&name) const +{ + return SharedLoggingLock(log, std::move(name), m_mutex, std::try_to_lock); +} + +inline UniqueLoggingLock GlobalLockable::tryLockToWrite(LogContext &log, std::string &&name) +{ + return UniqueLoggingLock(log, std::move(name), m_mutex, std::try_to_lock); +} + +inline UniqueLoggingLock GlobalLockable::lockToWrite(LogContext &log, std::string &&name, SharedLoggingLock &readLock) +{ + readLock.lock().unlock(); + return UniqueLoggingLock(log, std::move(name), m_mutex); +} + +} // namespace LibRepoMgr + +#endif // LIBREPOMGR_GLOBAL_LOCK_H diff --git a/librepomgr/namedlockable.cpp b/librepomgr/namedlockable.cpp deleted file mode 100644 index fc25d37..0000000 --- a/librepomgr/namedlockable.cpp +++ /dev/null @@ -1,9 +0,0 @@ -#include "./namedlockable.h" -#include "./logging.h" - -namespace LibRepoMgr { - -template struct NamedLock>; -template struct NamedLock>; - -} // namespace LibRepoMgr diff --git a/librepomgr/namedlockable.h b/librepomgr/namedlockable.h deleted file mode 100644 index b101fe2..0000000 --- a/librepomgr/namedlockable.h +++ /dev/null @@ -1,105 +0,0 @@ -#ifndef LIBREPOMGR_NAMED_LOCKABLE_H -#define LIBREPOMGR_NAMED_LOCKABLE_H - -#include -#include - -#include "./global.h" - -namespace LibRepoMgr { - -struct LogContext; - -template struct NamedLock { - template 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 &) -{ - return "shared"; -} -constexpr std::string_view lockName(std::unique_lock &) -{ - return "exclusive"; -} - -template -template -inline NamedLock::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)...); -} - -template inline NamedLock::~NamedLock() -{ - if (m_lock) { - m_lock.unlock(); - m_log("Released ", lockName(m_lock), " lock \"", m_name, "\"\n"); - } -} - -using SharedNamedLock = NamedLock>; -using UniqueNamedLock = NamedLock>; - -extern template struct LIBREPOMGR_EXPORT NamedLock>; -extern template struct LIBREPOMGR_EXPORT NamedLock>; - -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 diff --git a/librepomgr/serversetup.h b/librepomgr/serversetup.h index bb21e57..a169d6c 100644 --- a/librepomgr/serversetup.h +++ b/librepomgr/serversetup.h @@ -4,7 +4,7 @@ #include "./authentication.h" #include "./buildactions/buildaction.h" #include "./buildactions/buildactiontemplate.h" -#include "./namedlockable.h" +#include "./globallock.h" #include "../libpkg/data/config.h" #include "../libpkg/data/lockable.h" @@ -122,15 +122,15 @@ struct LIBREPOMGR_EXPORT ServiceSetup : public LibPkg::Lockable { } auth; struct LIBREPOMGR_EXPORT Locks { - [[nodiscard]] SharedNamedLock acquireToRead(LogContext &log, std::string &&lockName); - [[nodiscard]] UniqueNamedLock acquireToWrite(LogContext &log, std::string &&lockName); + [[nodiscard]] SharedLoggingLock acquireToRead(LogContext &log, std::string &&lockName); + [[nodiscard]] UniqueLoggingLock 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 m_locksByName; + std::unordered_map m_locksByName; } locks; void loadConfigFiles(bool restoreStateAndDiscardDatabases); @@ -149,13 +149,13 @@ inline std::shared_ptr ServiceSetup::BuildSetup::getBuildAction(Bui return id < actions.size() ? actions[id] : nullptr; } -inline SharedNamedLock ServiceSetup::Locks::acquireToRead(LogContext &log, std::string &&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)); } -inline UniqueNamedLock ServiceSetup::Locks::acquireToWrite(LogContext &log, std::string &&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)); diff --git a/librepomgr/tests/utils.cpp b/librepomgr/tests/utils.cpp new file mode 100644 index 0000000..ac407a6 --- /dev/null +++ b/librepomgr/tests/utils.cpp @@ -0,0 +1,71 @@ +#include "../globallock.h" +#include "../logging.h" + +#include +#include +#include + +#include +#include + +#include + +using namespace std; +using namespace CPPUNIT_NS; +using namespace CppUtilities; +using namespace CppUtilities::Literals; + +using namespace LibRepoMgr; + +class UtilsTests : public TestFixture { + CPPUNIT_TEST_SUITE(UtilsTests); + CPPUNIT_TEST(testGlobalLock); + CPPUNIT_TEST_SUITE_END(); + + void testGlobalLock(); + +public: + UtilsTests(); + void setUp() override; + void tearDown() override; + +private: +}; + +CPPUNIT_TEST_SUITE_REGISTRATION(UtilsTests); + +UtilsTests::UtilsTests() +{ +} + +void UtilsTests::setUp() +{ +} + +void UtilsTests::tearDown() +{ +} + +void UtilsTests::testGlobalLock() +{ + auto mutex = GlobalSharedMutex(); + auto sharedLock1 = std::shared_lock(mutex); + auto sharedLock2 = std::shared_lock(mutex); // locking twice is not a problem, also not from the same thread + auto thread1 = std::thread([&sharedLock1] { + sharedLock1.unlock(); // unlocking from another thread is ok + }); + auto thread2 = std::thread([&mutex] { mutex.lock(); }); + sharedLock2.unlock(); + thread1.join(); + thread2.join(); // thread2 should be able to acquire the mutex exclusively (and then terminate) + CPPUNIT_ASSERT_MESSAGE("try_lock_shared() returns false if mutex exclusively locked", !mutex.try_lock_shared()); + auto thread3 = std::thread([&mutex] { mutex.lock_shared(); }); + mutex.unlock(); + thread3.join(); // thread3 should be able to acquire the mutex (and then terminate) + CPPUNIT_ASSERT_MESSAGE("try_lock_shared() possible if mutex only shared locked", mutex.try_lock_shared()); + mutex.unlock_shared(); + CPPUNIT_ASSERT_MESSAGE("try_lock() returns false if mutex has still shared locked", !mutex.try_lock()); + mutex.unlock_shared(); + CPPUNIT_ASSERT_MESSAGE("try_lock() possible if mutex not locked", mutex.try_lock()); + mutex.unlock(); +}