Break circular links to std :: weak_ptr and the alias constructor: sound or problem?

Question

I have not yet found the following way to break cyclic links explained on any major C ++ forum / blog, for example, on GotW, so I wanted to ask if the technique is known, and what are its pro and disadvantages?

class Node : public std::enable_shared_from_this<Node> { public: std::shared_ptr<Node> getParent() { return parent.lock(); } // the getter functions ensure that "parent" always stays alive! std::shared_ptr<Node> getLeft() { return std::shared_ptr<Node>(shared_from_this(), left.get()); } std::shared_ptr<Node> getRight() { return std::shared_ptr<Node>(shared_from_this(), right.get()); } // add children.. never let them out except by the getter functions! public: std::shared_ptr<Node> getOrCreateLeft() { if(auto p = getLeft()) return p; left = std::make_shared<Node>(); left->parent = shared_from_this(); return getLeft(); } std::shared_ptr<Node> getOrCreateRight() { if(auto p = getRight()) return p; right = std::make_shared<Node>(); right->parent = shared_from_this(); return getRight(); } private: std::weak_ptr<Node> parent; std::shared_ptr<Node> left; std::shared_ptr<Node> right; }; 

From the outside, the Node user will not see the trick using the alias constructor in getLeft and getRight , but still the user can be sure that getParent always returns a non-empty common pointer, because all the pointers returned by p->get{Left,Right} save the object *p alive for the lifetime of the returned child pointer.

Am I forgetting something, or is this an obvious way to break circular references that have already been documented?

 int main() { auto n = std::make_shared<Node>(); auto c = n->getOrCreateLeft(); // c->getParent will always return non-null even if n is reset()! } 

Answer 1

shared_ptr<Node> returned by your getParent owns the parent, not the parent.

Thus, calling getParent again that shared_ptr can return empty (and null) shared_ptr . For instance:

 int main() { auto gp = std::make_shared<Node>(); auto p = gp->getOrCreateLeft(); auto c = p->getOrCreateLeft(); gp.reset(); p.reset(); // grandparent is dead at this point assert(c->getParent()); assert(!c->getParent()->getParent()); } 

(The inherited shared_from_this also skips the shared_ptr to which the node belongs and not its parent, but I believe that you can complicate its use with a private declaration and forbid it by contract.)