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:
Message::addChild
gets calledthisPtr
gets created with a reference count of 1childPtr->parent() != thisPtr
will be resolved to true
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 2Message::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.Message::setParent
and destroy parentPtr
decreasing the reference count of m_parentPtr
and thisPtr
by 1Message::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?...
- As
Message::setParent
gets executed,m_parentPtr = parentPtr;
will increasem_parentPtr
,parentPtr
and thusthisPtr
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
.