From 925f26633fb0a5ca8f11884c1bbf82e96c22e34b Mon Sep 17 00:00:00 2001 From: Maximilian Seidler <78690852+PaideiaDilemma@users.noreply.github.com> Date: Wed, 25 Jun 2025 19:41:24 +0200 Subject: [PATCH] memory: add CAtomicSharedPointer and CAtomicWeakPointer (#57) --- .gitignore | 2 + include/hyprutils/memory/Atomic.hpp | 364 ++++++++++++++++++++++++++++ tests/memory.cpp | 92 ++++++- 3 files changed, 456 insertions(+), 2 deletions(-) create mode 100644 include/hyprutils/memory/Atomic.hpp diff --git a/.gitignore b/.gitignore index 041d4c5..f5c0222 100644 --- a/.gitignore +++ b/.gitignore @@ -34,6 +34,7 @@ build/ .vscode/ .cache/ +.direnv/ .cmake/ CMakeCache.txt @@ -44,3 +45,4 @@ Makefile cmake_install.cmake compile_commands.json hyprutils.pc +.envrc diff --git a/include/hyprutils/memory/Atomic.hpp b/include/hyprutils/memory/Atomic.hpp new file mode 100644 index 0000000..f69acd5 --- /dev/null +++ b/include/hyprutils/memory/Atomic.hpp @@ -0,0 +1,364 @@ +#pragma once + +#include "./ImplBase.hpp" +#include "./SharedPtr.hpp" +#include "./WeakPtr.hpp" +#include + +/* + This header provides a thread-safe wrapper for Hyprutils shared pointer implementations. + Like with STL shared pointers, that does not mean that individual SP/WP objects can be shared across threads without synchronization. + It only means that the refcounting of the data is thread-safe. + + Should an Atomic SP/WP be shared across threads, calling a non-const member leads to a data race. + To avoid that, each thread should have thread-local SP/WP objects. + + Example: + We have a CAtomicSharedPointer member in a class. Suppose this member is accessed by multiple threads and is not constant. + In such a case we need external synchronization to ensure valid data access. + However, if we create a copy of this CAtomicWeakPointer member for each thread that accesses it, + then the references to the object will be counted in a thread-safe manner and it will be safe to lock a WP and to access the data in case of an SP. + In such an example, the inner data would need its own synchronization mechanism if it isn't constant itself. +*/ + +namespace Hyprutils::Memory { + namespace Atomic_ { + template + class impl : public Impl_::impl { + std::recursive_mutex m_mutex; + + public: + impl(T* data, bool lock = true) noexcept : Impl_::impl(data, lock) { + ; + } + + std::lock_guard lockGuard() { + return std::lock_guard(m_mutex); + } + }; + } + + // Forward declaration for friend + template + class CAtomicWeakPointer; + + template + class CAtomicSharedPointer { + template + using isConstructible = typename std::enable_if::value>::type; + template + using validHierarchy = typename std::enable_if&, X>::value, CAtomicSharedPointer&>::type; + + public: + explicit CAtomicSharedPointer(Impl_::impl_base* impl) noexcept : m_ptr(impl) {} + + CAtomicSharedPointer(const CAtomicSharedPointer& ref) { + if (!ref.m_ptr.impl_) + return; + + auto lg = ref.implLockGuard(); + m_ptr = ref.m_ptr; + } + + template > + CAtomicSharedPointer(const CAtomicSharedPointer& ref) { + if (!ref.m_ptr.impl_) + return; + + auto lg = ref.implLockGuard(); + m_ptr = ref.m_ptr; + } + + template > + CAtomicSharedPointer(CAtomicSharedPointer&& ref) noexcept { + std::swap(m_ptr, ref.m_ptr); + } + + CAtomicSharedPointer(CAtomicSharedPointer&& ref) noexcept { + std::swap(m_ptr, ref.m_ptr); + } + + CAtomicSharedPointer() noexcept { + ; // empty + } + + CAtomicSharedPointer(std::nullptr_t) noexcept { + ; // empty + } + + ~CAtomicSharedPointer() { + if (!m_ptr.impl_) + return; + + auto lg = implLockGuard(); + m_ptr.reset(); + } + + template + validHierarchy&> operator=(const CAtomicSharedPointer& rhs) { + if (m_ptr.impl_) { + auto lg = implLockGuard(); + m_ptr.reset(); + } + + if (!rhs.m_ptr.impl_) + return *this; + + auto lg = rhs.implLockGuard(); + m_ptr = rhs.m_ptr; + return *this; + } + + CAtomicSharedPointer& operator=(const CAtomicSharedPointer& rhs) { + if (this == &rhs) + return *this; + + if (m_ptr.impl_) { + auto lg = implLockGuard(); + m_ptr.reset(); + } + + if (!rhs.m_ptr.impl_) + return *this; + + auto lg = rhs.implLockGuard(); + m_ptr = rhs.m_ptr; + return *this; + } + + template + validHierarchy&> operator=(CAtomicSharedPointer&& rhs) noexcept { + std::swap(m_ptr, rhs.m_ptr); + return *this; + } + + CAtomicSharedPointer& operator=(CAtomicSharedPointer&& rhs) noexcept { + if (this == &rhs) + return *this; + + std::swap(m_ptr, rhs.m_ptr); + return *this; + } + + void reset() { + if (!m_ptr.impl_) + return; + + auto lg = implLockGuard(); + m_ptr.reset(); + } + + T& operator*() const { + return *m_ptr; + } + + T* operator->() const { + return m_ptr.get(); + } + + T* get() const { + return m_ptr.get(); + } + + operator bool() const { + return m_ptr; + } + + bool operator==(const CAtomicSharedPointer& rhs) const { + return m_ptr == rhs.m_ptr; + } + + bool operator()(const CAtomicSharedPointer& lhs, const CAtomicSharedPointer& rhs) const { + return lhs.m_ptr == rhs.m_ptr; + } + + unsigned int strongRef() const { + return m_ptr.impl_ ? m_ptr.impl_->ref() : 0; + } + + private: + std::lock_guard implLockGuard() const { + return ((Atomic_::impl*)m_ptr.impl_)->lockGuard(); + } + + CSharedPointer m_ptr; + + template + friend class CAtomicWeakPointer; + template + friend class CAtomicSharedPointer; + }; + + template + class CAtomicWeakPointer { + + template + using isConstructible = typename std::enable_if::value>::type; + template + using validHierarchy = typename std::enable_if&, X>::value, CAtomicWeakPointer&>::type; + + public: + CAtomicWeakPointer(const CAtomicWeakPointer& ref) { + if (!ref.m_ptr.impl_) + return; + + auto lg = ref.implLockGuard(); + m_ptr = ref.m_ptr; + } + + template > + CAtomicWeakPointer(const CAtomicWeakPointer& ref) { + if (!ref.m_ptr.impl_) + return; + + auto lg = ref.implLockGuard(); + m_ptr = ref.m_ptr; + } + + template > + CAtomicWeakPointer(CAtomicWeakPointer&& ref) noexcept { + std::swap(m_ptr, ref.m_ptr); + } + + CAtomicWeakPointer(CAtomicWeakPointer&& ref) noexcept { + std::swap(m_ptr, ref.m_ptr); + } + + CAtomicWeakPointer(const CAtomicSharedPointer& ref) { + if (!ref.m_ptr.impl_) + return; + + auto lg = ref.implLockGuard(); + m_ptr = ref.m_ptr; + } + + CAtomicWeakPointer() noexcept { + ; // empty + } + + CAtomicWeakPointer(std::nullptr_t) noexcept { + ; // empty + } + + ~CAtomicWeakPointer() { + if (!m_ptr.impl_) + return; + + auto lg = implLockGuard(); + m_ptr.reset(); + } + + template + validHierarchy&> operator=(const CAtomicWeakPointer& rhs) { + if (m_ptr.impl_) { + auto lg = implLockGuard(); + m_ptr.reset(); + } + + auto lg = rhs.implLockGuard(); + m_ptr = rhs.m_ptr; + return *this; + } + + CAtomicWeakPointer& operator=(const CAtomicWeakPointer& rhs) { + if (this == &rhs) + return *this; + + if (m_ptr.impl_) { + auto lg = implLockGuard(); + m_ptr.reset(); + } + + auto lg = rhs.implLockGuard(); + m_ptr = rhs.m_ptr; + return *this; + } + + template + validHierarchy&> operator=(CAtomicWeakPointer&& rhs) noexcept { + std::swap(m_ptr, rhs.m_ptr); + return *this; + } + + CAtomicWeakPointer& operator=(CAtomicWeakPointer&& rhs) noexcept { + if (this == &rhs) + return *this; + + std::swap(m_ptr, rhs.m_ptr); + return *this; + } + + void reset() { + if (!m_ptr.impl_) + return; + + auto lg = implLockGuard(); + m_ptr.reset(); + } + + T& operator*() const { + return *m_ptr; + } + + T* operator->() const { + return m_ptr.get(); + } + + T* get() const { + return m_ptr.get(); + } + + operator bool() const { + return m_ptr; + } + + bool operator==(const CAtomicWeakPointer& rhs) const { + return m_ptr == rhs.m_ptr; + } + + bool operator==(const CAtomicSharedPointer& rhs) const { + return m_ptr == rhs.m_ptr; + } + + bool operator()(const CAtomicWeakPointer& lhs, const CAtomicWeakPointer& rhs) const { + return lhs.m_ptr == rhs.m_ptr; + } + + bool expired() { + return m_ptr.expired(); + } + + bool valid() { + return m_ptr.valid(); + } + + CAtomicSharedPointer lock() const { + if (!m_ptr.impl_) + return {}; + + auto lg = implLockGuard(); + + if (!m_ptr.impl_->dataNonNull() || m_ptr.impl_->destroying() || !m_ptr.impl_->lockable()) + return {}; + + return CAtomicSharedPointer(m_ptr.impl_); + } + + private: + std::lock_guard implLockGuard() const { + return ((Atomic_::impl*)m_ptr.impl_)->lockGuard(); + } + + CWeakPointer m_ptr; + + template + friend class CAtomicWeakPointer; + template + friend class CAtomicSharedPointer; + }; + + template + static CAtomicSharedPointer makeAtomicShared(Args&&... args) { + return CAtomicSharedPointer(new Atomic_::impl(new U(std::forward(args)...))); + } +} diff --git a/tests/memory.cpp b/tests/memory.cpp index 118e8c8..8f33ca8 100644 --- a/tests/memory.cpp +++ b/tests/memory.cpp @@ -1,6 +1,11 @@ -#include #include +#include +#include +#include #include "shared.hpp" +#include +#include +#include #include using namespace Hyprutils::Memory; @@ -9,6 +14,87 @@ using namespace Hyprutils::Memory; #define WP CWeakPointer #define UP CUniquePointer +#define ASP CAtomicSharedPointer +#define AWP CAtomicWeakPointer +#define NTHREADS 8 +#define ITERATIONS 10000 + +static int testAtomicImpl() { + int ret = 0; + + { + // Using makeShared here could lead to invalid refcounts. + ASP shared = makeAtomicShared(0); + std::vector threads; + + threads.reserve(NTHREADS); + for (size_t i = 0; i < NTHREADS; i++) { + threads.emplace_back([shared]() { + for (size_t j = 0; j < ITERATIONS; j++) { + ASP strongRef = shared; + (*shared)++; + strongRef.reset(); + } + }); + } + + for (auto& thread : threads) { + thread.join(); + } + + // Actual count is not incremented in a thread-safe manner here, so we can't check it. + // We just want to check that the concurent refcounting doesn't cause any memory corruption. + shared.reset(); + EXPECT(shared, false); + } + + { + ASP shared = makeAtomicShared(0); + AWP weak = shared; + std::vector threads; + + threads.reserve(NTHREADS); + for (size_t i = 0; i < NTHREADS; i++) { + threads.emplace_back([weak]() { + for (size_t j = 0; j < ITERATIONS; j++) { + if (auto s = weak.lock(); s) { + (*s)++; + } + } + }); + } + + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + shared.reset(); + + for (auto& thread : threads) { + thread.join(); + } + + EXPECT(shared.strongRef(), 0); + EXPECT(weak.valid(), false); + + auto shared2 = weak.lock(); + EXPECT(shared, false); + EXPECT(shared2.get(), nullptr); + EXPECT(shared.strongRef(), 0); + EXPECT(weak.valid(), false); + EXPECT(weak.expired(), true); + } + + { // This tests recursive deletion. When foo will be deleted, bar will be deleted within the foo dtor. + class CFoo { + public: + AWP bar; + }; + + ASP foo = makeAtomicShared(); + foo->bar = foo; + } + + return ret; +} + int main(int argc, char** argv, char** envp) { SP intPtr = makeShared(10); SP intPtr2 = makeShared(-1337); @@ -62,5 +148,7 @@ int main(int argc, char** argv, char** envp) { EXPECT(*intPtr2AsUint, 10); EXPECT(*intPtr2, 10); + EXPECT(testAtomicImpl(), 0); + return ret; -} \ No newline at end of file +}