memory: make SP/ASP control blocks type agnostic (#79)

include/hyprutils/memory/Atomic.hpp

@@ -23,12 +23,11 @@
 
 namespace Hyprutils::Memory {
     namespace Atomic_ {
-        template <typename T>
+        class impl : public Impl_::impl_base {
-        class impl : public Impl_::impl<T> {
             std::recursive_mutex m_mutex;
 
           public:
-            impl(T* data, bool lock = true) noexcept : Impl_::impl<T>(data, lock) {
+            impl(void* data, DeleteFn deleter) noexcept : Impl_::impl_base(data, deleter) {
                 ;
             }
 
@@ -55,7 +54,13 @@ namespace Hyprutils::Memory {
         using validHierarchy = std::enable_if_t<std::is_assignable_v<CAtomicSharedPointer<T>&, X>, CAtomicSharedPointer&>;
 
       public:
-        explicit CAtomicSharedPointer(Impl_::impl_base* impl) noexcept : m_ptr(impl) {}
+        explicit CAtomicSharedPointer(T* object) noexcept : m_ptr(new Atomic_::impl(sc<void*>(object), _delete)) {
+            ;
+        }
+
+        CAtomicSharedPointer(Impl_::impl_base* impl) noexcept : m_ptr(impl) {
+            ;
+        }
 
         CAtomicSharedPointer(const CAtomicSharedPointer<T>& ref) {
             if (!ref.m_ptr.impl_)
@@ -141,7 +146,7 @@ namespace Hyprutils::Memory {
             // -> must unlock BEFORE reset
             // not last ref?
             // -> must unlock AFTER reset
-            auto& mutex = sc<Atomic_::impl<T>*>(m_ptr.impl_)->getMutex();
+            auto& mutex = sc<Atomic_::impl*>(m_ptr.impl_)->getMutex();
             mutex.lock();
 
             if (m_ptr.impl_->ref() > 1) {
@@ -152,7 +157,11 @@ namespace Hyprutils::Memory {
 
             if (m_ptr.impl_->wref() == 0) {
                 mutex.unlock(); // Don't hold the mutex when destroying it
-                m_ptr.reset();
+
+                m_ptr.impl_->destroy();
+                delete sc<Atomic_::impl*>(m_ptr.impl_);
+                m_ptr.impl_ = nullptr;
+
                 // mutex invalid
                 return;
             } else {
@@ -163,12 +172,18 @@ namespace Hyprutils::Memory {
                 // To avoid this altogether, keep a weak pointer here.
                 // This guarantees that impl_ is still valid after the reset.
                 CWeakPointer<T> guard = m_ptr;
-                m_ptr.reset();
+                m_ptr.reset(); // destroys the data
 
                 // Now we can safely check if guard is the last wref.
                 if (guard.impl_->wref() == 1) {
                     mutex.unlock();
-                    return; // ~guard destroys impl_ and mutex
+
+                    // destroy impl_ (includes the mutex)
+                    delete sc<Atomic_::impl*>(guard.impl_);
+                    guard.impl_ = nullptr;
+
+                    // mutex invalid
+                    return;
                 }
 
                 guard.reset();
@@ -205,8 +220,12 @@ namespace Hyprutils::Memory {
         }
 
       private:
+        static void _delete(void* p) {
+            std::default_delete<T>{}(sc<T*>(p));
+        }
+
         std::lock_guard<std::recursive_mutex> implLockGuard() const {
-            return sc<Atomic_::impl<T>*>(m_ptr.impl_)->lockGuard();
+            return sc<Atomic_::impl*>(m_ptr.impl_)->lockGuard();
         }
 
         CSharedPointer<T> m_ptr;
@@ -312,11 +331,13 @@ namespace Hyprutils::Memory {
             // -> must unlock BEFORE reset
             // not last ref?
             // -> must unlock AFTER reset
-            auto& mutex = sc<Atomic_::impl<T>*>(m_ptr.impl_)->getMutex();
+            auto& mutex = sc<Atomic_::impl*>(m_ptr.impl_)->getMutex();
             mutex.lock();
             if (m_ptr.impl_->ref() == 0 && m_ptr.impl_->wref() == 1) {
                 mutex.unlock();
-                m_ptr.reset();
+
+                delete sc<Atomic_::impl*>(m_ptr.impl_);
+                m_ptr.impl_ = nullptr;
                 // mutex invalid
                 return;
             }
@@ -375,7 +396,7 @@ namespace Hyprutils::Memory {
 
       private:
         std::lock_guard<std::recursive_mutex> implLockGuard() const {
-            return sc<Atomic_::impl<T>*>(m_ptr.impl_)->lockGuard();
+            return sc<Atomic_::impl*>(m_ptr.impl_)->lockGuard();
         }
 
         CWeakPointer<T> m_ptr;
@@ -387,7 +408,7 @@ namespace Hyprutils::Memory {
     };
 
     template <typename U, typename... Args>
-    static CAtomicSharedPointer<U> makeAtomicShared(Args&&... args) {
+    [[nodiscard]] inline CAtomicSharedPointer<U> makeAtomicShared(Args&&... args) {
-        return CAtomicSharedPointer<U>(new Atomic_::impl<U>(new U(std::forward<Args>(args)...)));
+        return CAtomicSharedPointer<U>(new U(std::forward<Args>(args)...));
     }
 }

include/hyprutils/memory/ImplBase.hpp

@@ -8,44 +8,71 @@ namespace Hyprutils {
         namespace Impl_ {
             class impl_base {
               public:
-                virtual ~impl_base() = default;
+                using DeleteFn = void (*)(void*);
 
-                virtual void         inc() noexcept         = 0;
+                impl_base(void* data, DeleteFn deleter, bool lock = true) noexcept : _lockable(lock), _data(data), _deleter(deleter) {
-                virtual void         dec() noexcept         = 0;
-                virtual void         incWeak() noexcept     = 0;
-                virtual void         decWeak() noexcept     = 0;
-                virtual unsigned int ref() noexcept         = 0;
-                virtual unsigned int wref() noexcept        = 0;
-                virtual void         destroy() noexcept     = 0;
-                virtual bool         destroying() noexcept  = 0;
-                virtual bool         dataNonNull() noexcept = 0;
-                virtual bool         lockable() noexcept    = 0;
-                virtual void*        getData() noexcept     = 0;
-            };
-
-            template <typename T>
-            class impl : public impl_base {
-              public:
-                impl(T* data, bool lock = true) noexcept : _lockable(lock), _data(data) {
                     ;
                 }
 
+                void inc() noexcept {
+                    _ref++;
+                }
+
+                void dec() noexcept {
+                    _ref--;
+                }
+
+                void incWeak() noexcept {
+                    _weak++;
+                }
+
+                void decWeak() noexcept {
+                    _weak--;
+                }
+
+                unsigned int ref() noexcept {
+                    return _ref;
+                }
+
+                unsigned int wref() noexcept {
+                    return _weak;
+                }
+
+                void destroy() noexcept {
+                    _destroy();
+                }
+
+                bool destroying() noexcept {
+                    return _destroying;
+                }
+
+                bool lockable() noexcept {
+                    return _lockable;
+                }
+
+                bool dataNonNull() noexcept {
+                    return _data != nullptr;
+                }
+
+                void* getData() noexcept {
+                    return _data;
+                }
+
+                ~impl_base() {
+                    destroy();
+                }
+
+              private:
                 /* strong refcount */
                 unsigned int _ref = 0;
                 /* weak refcount */
                 unsigned int _weak = 0;
                 /* if this is lockable (shared) */
-                bool        _lockable = true;
+                bool  _lockable = true;
 
-                T*          _data = nullptr;
+                void* _data = nullptr;
 
-                friend void swap(impl*& a, impl*& b) {
+                /* if the destructor was called,
-                    impl* tmp = a;
-                    a         = b;
-                    b         = tmp;
-                }
-
-                /* if the destructor was called, 
                    creating shared_ptrs is no longer valid */
                 bool _destroying = false;
 
@@ -63,56 +90,7 @@ namespace Hyprutils {
                     _destroying = false;
                 }
 
-                std::default_delete<T> _deleter{};
+                DeleteFn _deleter = nullptr;
-
-                //
-                virtual void inc() noexcept {
-                    _ref++;
-                }
-
-                virtual void dec() noexcept {
-                    _ref--;
-                }
-
-                virtual void incWeak() noexcept {
-                    _weak++;
-                }
-
-                virtual void decWeak() noexcept {
-                    _weak--;
-                }
-
-                virtual unsigned int ref() noexcept {
-                    return _ref;
-                }
-
-                virtual unsigned int wref() noexcept {
-                    return _weak;
-                }
-
-                virtual void destroy() noexcept {
-                    _destroy();
-                }
-
-                virtual bool destroying() noexcept {
-                    return _destroying;
-                }
-
-                virtual bool lockable() noexcept {
-                    return _lockable;
-                }
-
-                virtual bool dataNonNull() noexcept {
-                    return _data != nullptr;
-                }
-
-                virtual void* getData() noexcept {
-                    return _data;
-                }
-
-                virtual ~impl() {
-                    destroy();
-                }
             };
         }
     }

include/hyprutils/memory/SharedPtr.hpp

@@ -28,7 +28,7 @@ namespace Hyprutils {
 
             /* creates a new shared pointer managing a resource
                avoid calling. Could duplicate ownership. Prefer makeShared */
-            explicit CSharedPointer(T* object) noexcept : impl_(new Impl_::impl<T>(object)) {
+            explicit CSharedPointer(T* object) noexcept : impl_(new Impl_::impl_base(sc<void*>(object), _delete)) {
                 increment();
             }
 
@@ -140,6 +140,10 @@ namespace Hyprutils {
             Impl_::impl_base* impl_ = nullptr;
 
           private:
+            static void _delete(void* p) {
+                std::default_delete<T>{}(sc<T*>(p));
+            }
+
             /*
                 no-op if there is no impl_
                 may delete the stored object if ref == 0

include/hyprutils/memory/UniquePtr.hpp

@@ -22,7 +22,7 @@ namespace Hyprutils {
 
             /* creates a new unique pointer managing a resource
                avoid calling. Could duplicate ownership. Prefer makeUnique */
-            explicit CUniquePointer(T* object) noexcept : impl_(new Impl_::impl<T>(object, false)) {
+            explicit CUniquePointer(T* object) noexcept : impl_(new Impl_::impl_base(sc<void*>(object), [](void* p) { std::default_delete<T>{}(sc<T*>(p)); }, false)) {
                 increment();
             }
 

tests/memory.cpp

@@ -92,6 +92,38 @@ static int testAtomicImpl() {
         foo->bar      = foo;
     }
 
+    { // This tests destroying the data when storing the base class of a type
+        class ITest {
+          public:
+            size_t num = 0;
+            ITest() : num(1234) {};
+        };
+
+        class CA : public ITest {
+          public:
+            size_t num2 = 0;
+            CA() : ITest(), num2(4321) {};
+        };
+
+        class CB : public ITest {
+          public:
+            int num2 = 0;
+            CB() : ITest(), num2(-1) {};
+        };
+
+        ASP<ITest> genericAtomic = nullptr;
+        SP<ITest>  genericNormal = nullptr;
+        {
+            auto derivedAtomic = makeAtomicShared<CA>();
+            auto derivedNormal = makeShared<CA>();
+            genericAtomic      = derivedAtomic;
+            genericNormal      = derivedNormal;
+        }
+
+        EXPECT(!!genericAtomic, true);
+        EXPECT(!!genericNormal, true);
+    }
+
     return ret;
 }
 
@@ -113,6 +145,7 @@ int main(int argc, char** argv, char** envp) {
     EXPECT(intPtr.strongRef(), 1);
     EXPECT(*weak, 10);
     EXPECT(weak.expired(), false);
+    EXPECT(!!weak.lock(), true);
     EXPECT(*weakUnique, 420);
     EXPECT(weakUnique.expired(), false);
     EXPECT(intUnique.impl_->wref(), 1);
@@ -149,6 +182,5 @@ int main(int argc, char** argv, char** envp) {
     EXPECT(*intPtr2, 10);
 
     EXPECT(testAtomicImpl(), 0);
-
     return ret;
 }