this pointer and QSharedPointer

Question

I have a tree-node like class called Message which looks like this:

class Message
{
public:
    using Ptr = QSharedPointer<Message>;

public:
    explicit Message();
    explicit Message(Message::Ptr parentPtr);
    explicit Message(const Data &data, Message::Ptr parentPtr = Message::Ptr());

    void setParent(Message::Ptr parentPtr);
    Message::Ptr parent() const;
    bool hasParent() const;

    QSet<Message::Ptr> children() const;
    void setChildren(const QSet<Message::Ptr> &children);
    bool hasChildren() const;

    Data data() const;
    void setData(const Data &data);

private:
    void addChild(Message::Ptr childPtr);
    void removeChild(Message::Ptr childPtr);

private:
    Message::Ptr m_parentPtr;
    QSet<Message::Ptr> m_children;
    Data m_data;
};

This class can have a parent and a set of children. I have a problem when I implemented the addChild and setParent member functions:

void Message::addChild(Message::Ptr childPtr)
{
    if (!m_children.contains(childPtr)) {
        m_children.insert(childPtr);
    }

    Message::Ptr thisPtr(this);

    if (childPtr->parent() != thisPtr) {
        childPtr->setParent(thisPtr);
    }
}

void Message::setParent(Message::Ptr parentPtr)
{
    if (m_parentPtr != parentPtr) {
        m_parentPtr = parentPtr;

        m_parentPtr->addChild(Message::Ptr(this));
    }
}

What I expect will happen:

1. Message::addChild gets called
2. thisPtr gets created with a reference count of 1
3. childPtr->parent() != thisPtr will be resolved to true
4. childPtr->setParent(thisPtr);, Message::setParent gets executed and thisPtr reference count will increase by 1 as a copy of the shared pointer is created. Now thisPtr has a reference count of 2
5. As Message::setParent gets executed, m_parentPtr = parentPtr; will increase m_parentPtr, parentPtr and thus thisPtr reference counts by 1; these 3 smart pointers now have a reference count of 3.
6. Execution exits Message::setParent and destroy parentPtr decreasing the reference count of m_parentPtr and thisPtr by 1
7. Execution returns to Message::addChild. Now reference count of thisPtr is 2.

What actually happens:

When execution exits the if statement in Message::addChild thisPtr reference count decreases again by 1, leaving thisPtr with a reference count of 1. This makes everything break as when execution exists Message::addChild, thisPtr gets destroyed, thus this deleted.

My question:

Why does thisPtr reference count decreases again by when execution exits the if statement in Message::addChild or what actually happens there?...

Here is how it runs in the debugger:

Answer 1

5. As Message::setParent gets executed, m_parentPtr = parentPtr; will increase m_parentPtr, parentPtr and thus thisPtr reference counts by 1; these 3 smart pointers now have a reference count of 3.

5.1. Then, setParent constructs a temporary shared pointer to the child with reference count 1 and calls addChild on the parent:

m_parentPtr->addChild(Message::Ptr(this));

5.2. addChild creates a shared pointer to the parent with reference count 1:

Message::Ptr thisPtr(this);

5.3. addChild returns, destroying that shared pointer of 5.2, which destroys the parent, which destroys the parent's QSet<Message::Ptr> m_children member.

5.4. The temporary shared pointer of 5.1 is destroyed, which destroys the child.

More generally, you have a cyclic reference: parents own children, and children own their parents, which is a recipe for memory leaks and use-after-delete bugs. Constructing new shared pointers owning raw pointers already owned by other shared pointers is a recipe for double-delete and use-after-delete bugs; the shared pointers won't know about each other, their reference counts will vary independently. You should investigate QWeakPointer to break the cycle and QEnableSharedFromThis to safely obtain a shared pointer to *this.