2
\$\begingroup\$

I needed a reference counted smart pointer for my project, and for some reason early in my project, I decided that I did not like the std::shared_ptr. I can't remember the exact reasons why, but I do remember there was a reason for it. It fell short in some area, but for the life of me I can't remember what it was. Reasons for not using std::shared_ptr aside, I was hoping to get my own implementation reviewed and see if there were any obvious mistakes and errors that you guys could pick out. I used a couple of references to implement this (cited in the file header block) so I'm hoping it's at least somewhat solid, however I know there are some areas that can be improved and some areas where I had to get a little tricky ("clever code... isn't" and all that) and was hoping you guys could show me where that would be.

There is also a weak_ptr implementation as well as a std::hash specialization for the RefPtr and a specialization for the rttr library, which I make use of in my project.

///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
//
//  RefPtr
//
//  Reference counted smart pointer.
//  Adapted from http://www.acodersjourney.com/implementing-smart-pointer-using-reference-counting/\
//  With some tidbits from https://codereview.stackexchange.com/questions/99087/alternate-weak-pointer-implementation
//
//  Season to taste.
//
///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
//  Copyright (c) Project Firestorm 2018
///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
#ifndef LIBCORE_REFPTR_H_
#define LIBCORE_REFPTR_H_
#pragma once

#include <libCore/libCore.h>
#include <rttr/wrapper_mapper.h>

OPEN_NAMESPACE(Firestorm);

class RefCount
{
    template <class T> friend class RefPtr;
    template <class T> friend class WeakPtr;
public:
    template <class T>
    RefCount(T* object)
    :_object(static_cast<void*>(object))
    {
    }

    template<class T>
    T* _get_ptr() const { return static_cast<T*>(_object); }

private:
    uint32_t _strongCount{ 0 };
    uint32_t _weakCount{ 0 };
    void* _object;
};

// Dummy type that allows the program to compile when the static_assert hits in RefPtr.
struct _RefPtr_DummyType { virtual ~_RefPtr_DummyType() {} };

template<class T>
class RefPtr
{
    template <class T> friend class WeakPtr;
    /*static_assert(std::has_virtual_destructor<
        std::conditional<std::is_polymorphic<T>::value, T, _RefPtr_DummyType>::type
    >::value, "Objects used in a RefPtr must have a virtual destructor");*/

public:
    using DeleteExpr = std::function<void(T*)>;
    typedef T element_type;
    typedef T* pointer_type;

    RefPtr()
    : _object(nullptr)
    , _count(nullptr)
    , _deleter(nullptr)
    {
    }

    RefPtr(std::nullptr_t)
    : _object(nullptr)
    , _count(nullptr) 
    , _deleter(nullptr)
    {
    }

    RefPtr(T* object)
    : _object(object)
    , _count(new RefCount(object))
    , _deleter(nullptr)
    {
        assert(_count && _count->_object == _object);
        ++_count->_strongCount;
    }

    template<class Deleter>
    RefPtr(T* object, Deleter deleter)
    : _object(object)
    , _count(new RefCount(object))
    , _deleter(deleter)
    {
        assert(_count && _count->_object == _object);
        ++_count->_strongCount;
    }

    RefPtr(const RefPtr<T>& other)
    : _object(other._object)
    , _count(other._count)
    , _deleter(other._deleter)
    {
        assert(_count && _count->_object == _object);
        ++_count->_strongCount;
    }

    // polymorphic support
    template <class Subclass_t>
    RefPtr(const RefPtr<Subclass_t>& other)
    : _object(other._Get_Ptr())
    , _count(other._Get_Count())
    {
        static_assert(std::is_base_of<T, Subclass_t>::value,
            "the RefPtr passed to the casting constructor of RefPtr must hold a type that is a subclass of the held type");

        ++_count->_strongCount;

        // This is IWhatever, other is class Whatever : public IWhatever. ya feel?
        auto otherDel = other._Get_Deleter();
        if(otherDel)
        {
            // what has science done?
            _deleter = [del = otherDel](T* ptr) {
                if(ptr)
                    // so in order to call the other deleter what the superclass defined, we need to wrap it in our own
                    // that supports our needs.
                    del(reinterpret_cast<Subclass_t*>(ptr));
            };
        }
    }

    // polymorphic support with provided deleter.
    template <class Subclass_t, class Del>
    RefPtr(const RefPtr<Subclass_t>& other, Del deleter)
    : _object(other._Get_Ptr())
    , _count(other._Get_Count())
    , _deleter(deleter)
    {
        static_assert(std::is_base_of<T, Subclass_t>::value,
            "the RefPtr passed to the casting constructor of RefPtr must hold a type "
            "that is a subclass of the held type");
        ++_count->_strongCount;
    }

    // polymorphic support for raw pointer
    template <class Subclass_t>
    RefPtr(Subclass_t* ptr)
    : _object(static_cast<T*>(ptr))
    , _count(new RefCount(_object))
    {
        static_assert(std::is_base_of<T, Subclass_t>::value,
            "the RefPtr passed to the casting constructor of RefPtr must hold a type "
            "that is a subclass of the held type");
        ++_count->_strongCount;
    }

    // polymorphic support for raw pointer
    template <class Subclass_t, class Del>
    RefPtr(Subclass_t* ptr, Del deleter)
    : _object(static_cast<T*>(ptr))
    , _count(new RefCount(_object))
    , _deleter(deleter)
    {
        static_assert(std::is_base_of<T, Subclass_t>::value,
            "the RefPtr passed to the casting constructor of RefPtr must hold a type "
            "that is a subclass of the held type");
        ++_count->_strongCount;
    }

    // polymorphic move
    template<class Subclass_t>
    RefPtr(RefPtr<Subclass_t>&& other)
    : _object(other._object)
    , _count(other._count)
    {
        static_assert(std::is_base_of<T, Subclass_t>::value,
            "the RefPtr passed to the casting move constructor of RefPtr must hold a type "
            "that is a subclass of the held type");
        assert(_count && _count->_object == _object);

        // This is IWhatever, other is class Whatever : public IWhatever. ya feel?
        auto otherDel = other._Get_Deleter();
        if(otherDel)
        {
            // what has science done?
            _deleter = [del = otherDel](T* ptr) {
                if(ptr)
                    // so in order to call the other deleter what the superclass defined, we need to wrap it in our own
                    // that supports our needs.
                    del(reinterpret_cast<Subclass_t*>(ptr));
            };
        }

        // assumes ownership
        other._count = nullptr;
        other._object = nullptr;
        other._deleter = nullptr;
    }

    RefPtr(RefPtr<T>&& other)
    : _object(other._object)
    , _count(other._count)
    , _deleter(other._deleter)
    {
        assert(_count && "counter block was nullptr");
        T* o = _count->_get_ptr<T>();
        assert(o == _object && "counter block held a different object for some reason");

        // assumes ownership
        other._count = nullptr;
        other._object = nullptr;
        other._deleter = nullptr;
    }

    virtual ~RefPtr()
    {
        if(_count)
            --_count->_strongCount;
        DoCleanup();
    }

    RefPtr<T>& operator=(const RefPtr<T>& other)
    {
        if(this != &other)
        {
            // decrement the strong count of the existing counter.
            if(_count)
                --_count->_strongCount;
            DoCleanup();
            // any weak pointers that still hold the control block from this RefPtr will clean them up as
            // expected.

            // copy over the stuff.
            _object = other._object;
            _count = other._count;
            _deleter = other._deleter;

            // add a reference for the new counter.
            assert(_count);
            ++_count->_strongCount;
        }
        return *this;
    }

    RefPtr<T>& operator=(RefPtr<T>&& other)
    {
        if(this != &other)
        {
            _object = other._object;
            _count = other._count;

            assert(_count && _count->_object == other._object);

            other._count = nullptr;
            other._object = nullptr;
            other._deleter = nullptr;
        }
        return *this;
    }

    T* Get() const { return _object; }
    T& operator*() { return *_object; }
    T* operator->() { return _object; }

    const T* operator->() const { return _object; }

    bool operator==(const RefPtr<T>& other) const
    {
        return _object == other._object;
    }

    bool operator==(std::nullptr_t) const
    {
        return _object == nullptr;
    }

    bool operator!=(const RefPtr<T>& other) const
    {
        return _object != other._object;
    }

    bool operator!=(std::nullptr_t) const
    {
        return _object != nullptr;
    }

    operator bool() const
    {
        return _object != nullptr;
    }

    uint32_t GetCount() const
    {
        FIRE_ASSERT(_count);
        return _count->_strongCount;
    }

    uint32_t GetWeakCount() const
    {
        FIRE_ASSERT(_count);
        return _count->_weakCount;
    }

    template<class U=T>
    U*         _Get_Ptr()     const { return static_cast<U*>(_object); }
    RefCount*  _Get_Count()   const { return _count; }
    DeleteExpr _Get_Deleter() const { return _deleter; }

private:
    RefPtr(RefCount* refCount)
    : _object(static_cast<T*>(refCount->_object))
    , _count(refCount)
    {
        FIRE_ASSERT(_count);
        ++_count->_strongCount;
    }

    void DoCleanup()
    {
        // if there are no more strong references...
        if (_count && _count->_strongCount == 0)
        {
            // then delete the object.
            // _object->~T(); // call the destructor so that we can delete it properly.
            // delete _object;
            // _object = nullptr;
            if(_deleter) _deleter(_object);
            else delete _object;

            // and null out the counter's pointer as well.
            _count->_object = nullptr;

            // if there are no more weak references, then we can delete the counter as well.
            if(_count->_weakCount == 0)
            {
                delete _count;
                _count = nullptr;
            }
        }
    }

    T*         _object{ nullptr };
    RefCount*  _count{ nullptr };
    DeleteExpr _deleter{ nullptr };
};

template <class T>
class WeakPtr
{
public:
    typedef T* T_ptr;

    WeakPtr() : _count(nullptr) {}
    WeakPtr(const RefPtr<T>& ptr)
    : _count(ptr._count)
    {
        FIRE_ASSERT(_count);
        ++_count->_weakCount;
    }

    WeakPtr(const WeakPtr<T>& ptr)
    : _count(ptr._count)
    {
        FIRE_ASSERT(_count);
        ++_count->_weakCount;
    }

    template <class Subclass_t>
    WeakPtr(const RefPtr<Subclass_t>& ptr)
    : _count(ptr._count)
    {
        FIRE_ASSERT(_count);
        static_assert(std::is_base_of<T, Subclass_t>::value, 
            "the pointer passed to the casting constructor must be a subclass of the type held by the WeakPtr");
        ++_count->_weakCount;
    }

    ~WeakPtr()
    {
        if(_count)
        {
            // if there are no more weak references and there's no object (released by the RefPtr)
            // then we can delete the control block.
            if(--_count->_weakCount == 0 && !_count->_object)
            {
                delete _count;
                _count = nullptr;
            }
        }
    }

    bool operator==(WeakPtr<T>& other)
    {
        return other._count == _count;
    }

    bool operator==(RefPtr<T>& other)
    {
        return other._count == _count;
    }

    bool operator==(std::nullptr_t) const
    {
        return _counter && _counter->_object == nullptr;
    }

    bool operator!=(std::nullptr_t) const
    {
        return _counter && _counter->_object != nullptr;
    }

    template <class Subclass_t>
    WeakPtr<T>& operator=(const RefPtr<Subclass_t>& ptr)
    {
        static_assert(std::is_base_of<T, Subclass_t>::value,
            "the RefPtr passed to the casting constructor must hold an instance of a subclass of the type held by the WeakPtr");

        // decrement the weak ref count of the current control block if we have it
        // and then clean it up if we need to.
        if(_count)
        {
            if(--_count->_weakCount == 0 && !_count->_object)
            {
                delete _count;
                _count = nullptr;
            }
        }

        // increment the weak ref count of the new control block.
        _count = ptr._count;
        FIRE_ASSERT(_count);
        ++_count->_weakCount;
        return *this;
    }

    WeakPtr<T>& operator=(const RefPtr<T>& obj)
    { 
        // decrement the weak ref count of the current control block if we have it
        // and then clean it up if we need to.
        if(_count)
        {
            if(--_count->_weakCount == 0 && !_count->_object)
            {
                delete _count;
                _count = nullptr;
            }
        }

        // increment the weak ref count of the new control block.
        _count = obj._count;
        FIRE_ASSERT(_count);
        ++_count->_weakCount;
        return *this; 
    }

    RefPtr<T> Lock() const
    {
        return RefPtr<T>(_count);
    }

    operator bool() const
    {
        return _count->_object != nullptr;
    }
private:
    RefCount* _count;
};

CLOSE_NAMESPACE(Firestorm);

OPEN_NAMESPACE(rttr);

template<typename T>
struct wrapper_mapper<Firestorm::RefPtr<T>>
{
    using wrapped_type = decltype(Firestorm::RefPtr<T>().Get());
    using type = Firestorm::RefPtr<T>;

    static RTTR_INLINE wrapped_type get(const type& obj)
    {
        return obj.Get();
    }

    static RTTR_INLINE type create(const wrapped_type& t)
    {
        return type(t);
    }

    template<typename U>
    static Firestorm::RefPtr<U> convert(const type& source, bool& ok)
    {
        if(auto p = rttr_cast<typename Firestorm::RefPtr<U>::element_type*>(source.Get()))
        {
            ok = true;
            return Firestorm::RefPtr<U>(p);
        }
        else
        {
            ok = false;
            return Firestorm::RefPtr<U>(nullptr);
        }
    }
};

CLOSE_NAMESPACE(rttr);

OPEN_NAMESPACE(std);

template<class T>
struct hash<Firestorm::RefPtr<T>>
{
    size_t operator()(const Firestorm::RefPtr<T>& ptr) const
    {
        return std::hash<Firestorm::RefPtr<T>::pointer_type>()(ptr.Get());
    }
};
CLOSE_NAMESPACE(std);

#endif

I'm also including my unit tests, however it uses my own unit testing implementation so you'd have to translate it over to whatever unit test framework you fancy. So far it passes all of these unit tests, however I know there are some cases I'd probably want to test for that I haven't thought of yet.

using std::cout;
using std::cin;
using std::endl;

using namespace Firestorm;

struct TestObject
{
    TestObject(const Function<void()>& deletionCallback)
        : _callback(deletionCallback)
    {
    }
    virtual ~TestObject()
    {
        _callback();
    }
    Function<void()> _callback;
};

struct TestPolyObject : public TestObject
{
    TestPolyObject(const std::function<void()>& deletionCallback)
        : TestObject(deletionCallback)
    {
    }
};

RefPtr<TestHarness> libCorePrepareHarness(int argc, char** argv)
{
    RefPtr<TestHarness> h(new TestHarness("libCore Tests"));

    //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
    // RefPtr Testing
    //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
    h->It("RefPtr should delete the object when it falls out of scope", [](TestCase& t) {
        bool objectDeleted = false;
        {
            auto callback = [&objectDeleted]() { objectDeleted = true; };
            RefPtr<TestObject> object(new TestObject(callback));
            t.Assert(object.GetCount() == 1, "the ref count was not the expected value");
        }
        t.Assert(objectDeleted, "the object was not deleted");
    });

    h->It("RefPtr should use the deleter when one is provided", [](TestCase& t) {
        bool objectDeleted = false;
        bool deleterCalled = false;
        {
            auto callback = [&objectDeleted]() { objectDeleted = true; };
            RefPtr<TestObject> object(new TestObject(callback), [&deleterCalled](TestObject* ptr) {
                if(ptr)
                    delete ptr;
                deleterCalled = true;
            });
            t.Assert(object.GetCount() == 1, "the ref count was not the expected value");
        }
        t.AssertIsTrue(objectDeleted, "the object was not deleted");
        t.AssertIsTrue(deleterCalled, "the deleter was not called");
    });

    h->It("RefPtr should use the deleter when one is provided, even when polymorphism is in play", [](TestCase& t) {
        bool objectDeleted = false;
        bool deleterCalled = false;
        {
            auto callback = [&objectDeleted]() { objectDeleted = true; };
            RefPtr<TestObject> object(new TestPolyObject(callback), [&deleterCalled](TestObject* ptr) {
                if (ptr)
                    delete ptr;
                deleterCalled = true;
            });
            t.Assert(object.GetCount() == 1, "the ref count was not the expected value");
        }
        t.AssertIsTrue(objectDeleted, "the object was not deleted");
        t.AssertIsTrue(deleterCalled, "the deleter was not called");
    });

    h->It("RefPtr should use the deleter when one is provided, even when polymorphism is in play "
          "and the base ptr was copy constructed", [](TestCase& t) {
              {
                  bool objectDeleted = false;
                  bool deleterCalled = false;
                  {
                      auto callback = [&objectDeleted]() { objectDeleted = true; };
                      RefPtr<TestPolyObject> object(new TestPolyObject(callback), [&deleterCalled](TestPolyObject* ptr) {
                          if (ptr)
                              delete ptr;
                          deleterCalled = true;
                      });
                      t.Assert(object.GetCount() == 1, "the ref count was not the expected value");

                      {
                          // copy construct.
                          RefPtr<TestObject> testBaseObject(object);
                          t.Assert(object.GetCount() == 2, "the ref count was not the expected value");
                          t.Assert(testBaseObject.GetCount() == 2, "the ref count was not the expected value");
                      }
                      t.Assert(object.GetCount() == 1, "the ref count was not the expected value");
                      t.AssertIsFalse(deleterCalled, "the deleter was called prematurely");

                      RefPtr<TestObject> testBaseObject(object);
                      t.Assert(object.GetCount() == 2, "the ref count was not the expected value");
                      t.Assert(testBaseObject.GetCount() == 2, "the ref count was not the expected value");
                  }
                  t.AssertIsTrue(objectDeleted, "the object was not deleted");
                  t.AssertIsTrue(deleterCalled, "the deleter was not called");
              }
              {
                  bool objectDeleted = false;
                  bool deleterCalled = true;
                  {
                      auto callback = [&objectDeleted]() { objectDeleted = true; };
                      RefPtr<TestObject> ptrBase(new TestPolyObject(callback), [&deleterCalled](TestObject* ptr) {
                          if(ptr)
                              delete ptr;
                          deleterCalled = true;
                      });
                  }
                  t.AssertIsTrue(deleterCalled, "the deleter was not called on a polymorphic type");
              }
    });

    h->It("weak pointers should not prolong the lifetime of objects managed by a shared ptr", [](TestCase& t) {
        bool objectDeleted = false;
        auto callback = [&objectDeleted]() { objectDeleted = true; };
        WeakPtr<TestObject> weak;
        {
            RefPtr<TestObject> object(new TestObject(callback));
            t.Assert(object.GetCount() == 1, "the ref count was not the expected value");
            weak = object;
            t.Assert(object.GetWeakCount() == 1, "the weak ref count was not the expected value");
            t.Assert(object.GetCount() == 1, "the ref count was not the expected value");
        }
        t.Assert(!weak, "the weak pointer still holds on to a valid object");
        t.Assert(objectDeleted, "the object was not deleted");
    });

    h->It("polymorphic types should be allowable by the RefPtr", [](TestCase& t) {
        bool objectDeleted = false;

        auto callback = [&objectDeleted]() { objectDeleted = true; };
        {
            RefPtr<TestObject> object(new TestPolyObject(callback));
            t.Assert(object.GetCount() == 1, "the ref count was not the expected value");
        }

        t.Assert(objectDeleted, "the object was not deleted");
    });

    h->It("weak pointers should also be allowed to hold polymorphic types", [](TestCase& t) {
        bool objectDeleted = false;
        auto callback = [&objectDeleted]() { objectDeleted = true; };
        WeakPtr<TestObject> weak;
        {

            RefPtr<TestPolyObject> object(new TestPolyObject(callback));
            t.Assert(object.GetCount() == 1, "the ref count was not the expected value");

            weak = object;

            t.Assert(object.GetWeakCount() == 1, "the weak ref count was not the expected value");
            t.Assert(object.GetCount() == 1, "the ref count was not the expected value");
        }
        t.Assert(!weak, "the weak pointer still holds on to a valid object");
        t.Assert(objectDeleted, "the object was not deleted");
    });

    return h;
}
\$\endgroup\$
  • \$\begingroup\$ Why have a copy of _object in both RefCount and RefPtr? \$\endgroup\$ – G. Sliepen Sep 21 '18 at 21:13
  • \$\begingroup\$ That's a good point actually. I believe that my thinking was that since I'm storing it as a void* there for polymorphism support, it would cut down on the number of casts to the actual type in RefPtr that have to be done. Also, calling delete on void* wont invoke the destructors. \$\endgroup\$ – Super Cyb0rg Sep 21 '18 at 23:07
  • \$\begingroup\$ Why do you allow to have RefPtr's to same object but with different deleters? (_deleter inside RefPtr and not inside RefCount + RefPtr(const RefPtr<Subclass_t>& other, Del deleter)) \$\endgroup\$ – firda Sep 24 '18 at 8:33
  • \$\begingroup\$ Are you really using a compiler (or expecting this to be compiled in an environment) where namespaces are not supported? OPEN_NAMESPACE(Firestorm); \$\endgroup\$ – Martin York Sep 25 '18 at 23:21
  • \$\begingroup\$ It's not really an intrusive pointer (it does not depend on the object to implement inc/dec). It is more like a std::shared_ptr \$\endgroup\$ – Martin York Sep 25 '18 at 23:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.