Breaking cyclic references with std::weak_ptr and alias constructor: sound or problematic?

Question

I have not yet found the following way of breaking cyclic references explained on any major C++ forum/blog, like on GotW, so I wanted to ask whether the technique is known, and what are its pro and cons?

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 user of Node will not notice the trick with using the aliasing constructor in getLeft and getRight, but still the user can be sure that getParent always returns a non-empty shared pointer, because all pointers returned by p->get{Left,Right} keep the object *p alive for the lifetime of the returned child pointer.

Am I overlooking something here, or is this an obvious way to break cyclic references that has been documented already?

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

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

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

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 passes out shared_ptrs that owns the node rather than its parent, but I suppose you can make it harder to mess up by a private using declaration and ban it by contract.)