-1

I am currently reading the "3D Game Engine Architecture" book by David H. Eberly, and decided to implement my own little reference counting smart pointer. I have mostly followed his implementation, but I am experiencing a problem with my implementation.

I created a function called 'CreateRef' which returns a Pointer. All is well when I use this function in the same scope as the object I have created, but the moment I put the object in the global scope it destroys the object right after creation.

class Object
{
public:
    void IncrementReferences()
    {
        ++m_References;
    }
    void DecrementReferences()
    {
        if(--m_References == 0) delete this;
    }
    int GetReferenceCount() const { return m_References; }

private:
    int m_References = 0;
};
template<class T>
class Pointer
{
public:
    //costr and destr
    Pointer(T* pObject = nullptr)
    {
        m_pObject = pObject;
        if (m_pObject)
            m_pObject->IncrementReferences();
    }

    Pointer(const Pointer& rPointer)
    {
        m_pObject = rPointer.m_pObject;
        if (m_pObject)
            m_pObject->IncrementReferences();
    }

    ~Pointer()
    {
        if (m_pObject)
            m_pObject->DecrementReferences();
    }

    // implicit conversions
    operator T* () const
    {
        return m_pObject;
    }
    T& operator* () const
    {
        return *m_pObject;
    }
    T* operator-> () const
    {
        return m_pObject;
    }

    // Assignment
    Pointer& operator= (T* pObject)
    {
        if (m_pObject != pObject)
        {
            if (pObject)
                pObject->IncrementReferences();

            if (m_pObject)
                m_pObject->DecrementReferences();

            m_pObject = pObject;
        }
        return *this;
    }
    Pointer& operator= (const T* rReference)
    {
        if (m_pObject != rReference)
        {
            if (rReference)
                rReference->IncrementReferences();

            if (m_pObject)
                m_pObject->DecrementReferences();

            m_pObject = rReference;
        }
        return *this;
    }

    // Comparisons
    bool operator== (T* pObject) const { return m_pObject == pObject; }
    bool operator!= (T* pObject) const { return m_pObject != pObject; }
    bool operator== (const Pointer& rReference) const { return m_pObject == rReference.m_pObject; }
    bool operator!= (const Pointer& rReference) const { return m_pObject != rReference.m_pObject; }

protected:
    // The shared object
    T* m_pObject;
};
template<typename T>
using Ref = Pointer<T>;

template<typename T, typename ...Args>
constexpr Ref<T> CreateRef(Args&&... args)
{
    return Ref<T>(new T(args...));
}

Main

static Ref<Person> person = nullptr; // Doesn't work like this

static void DoSomething()
{
    person = CreateRef<Person>("Name");
    std::cout << "References " << person->GetReferenceCount() << std::endl;

    Ref<Person> newPerson = person;
    std::cout << "References " << newPerson->GetReferenceCount() << std::endl;
}

int main()
{
    DoSomething();

    std::cout << person->GetReferenceCount();
}

I have a feeling I am doing something wrong with the 'Pointer' class but I can't quite understand what I am missing.

TsvetelinD
  • 39
  • 3
  • 2
    Fire up your debugger. Put breakpoints in the `IncrementReferences` and `DecrementReferences()` methods. See where the breakpoints hit, what the `this` pointer is, and what the call stack is when they do. If you don't know how to use a debugger, this is a great time to learn. Perfect sort of problem made a ton easier with a debugger. Post the info you glean from the debugger here as well, we can help. – Tumbleweed53 Dec 21 '20 at 00:53
  • So I tried debugging, and this is how the callstack looks when it reaches both [IncrementReference](https://paste.pics/B3J5R) and [DecrementReference](https://paste.pics/B3J67). Both of these are called consecutively on the first line in the "DoSomething()" function – TsvetelinD Dec 21 '20 at 01:04
  • 2
    You `Pointer` class template needs a copy assignment. – Dietmar Kühl Dec 21 '20 at 01:12
  • 3
    @TsvetelinD `Pointer& operator= (const Pointer& rPointer)` -- Your `Pointer` class is missing this function, thus violates the [rule of 3](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) – PaulMcKenzie Dec 21 '20 at 01:18

1 Answers1

0

Thanks for the help. I found two solutions to my problem.

First solution is to add a copy assignment operator to the Pointer class.

Pointer& operator= (Pointer& rPointer)
    {
        if (m_pObject != rPointer.m_pObject)
        {
            if (rPointer)
                rPointer.m_pObject->IncrementReferences();

            if (m_pObject)
                m_pObject->DecrementReferences();

            m_pObject = rPointer.m_pObject;
        }
        return *this;
    }

Another solution (albeit not what I was looking for) was to change the return type of the CreateRef() function to be a T*

template<typename T, typename ...Args>
constexpr T* CreateRef(Args&&... args)
{
    return new T(args...);
}
TsvetelinD
  • 39
  • 3
  • 2
    *First solution is to add another copy assignment operator* -- You didn't have any copy assignment operator up until you added the code above. What you previously did was overload `=`, but none of those previous functions were copy-assignment. – PaulMcKenzie Dec 21 '20 at 01:20
  • Thanks for the clarity! Still have a big way to go before I can confidently say I'm decent at c++ – TsvetelinD Dec 21 '20 at 01:23