2

I have a question about the behaviour of shared_ptr.reset().

In this scenario I have a cyclic reference with the following classes. I have a book and an owner, which both have std::shared_ptrs to each other, creating a cyclic reference.

Book.h

#pragma once

class Owner;

class Book
{
public:
    Book(std::string title);
    ~Book();
    void OutputDetails();
    void SetOwner(std::shared_ptr<Owner> owner);
    void OutputOwnerInformation();
private:
    std::string m_title;
    std::shared_ptr<Owner> m_owner; // Book hangs onto the owner and creates a circular dependency
};

Book.cpp

#include "stdafx.h"
#include <iostream>
#include "Book.h"
#include "Owner.h"

Book::Book(std::string title) : m_title(title) {}

Book::~Book() {
    std::cout << "Book Destroyed" << std::endl;
}

void Book::SetOwner(std::shared_ptr<Owner> owner) {
    m_owner = owner; // strong reference
}

void Book::OutputOwnerInformation() {
    std::cout << "Owner is: " << m_owner->GetName() << std::endl;
}

void Book::OutputOwnerInformation() {
    std::cout << "Owner is: " << m_owner->GetName() << std::endl;
}

Owner.h

#pragma once

class Book; // To avoid circular #includes

class Owner
{
public:
    Owner(std::string name, std::shared_ptr<Book> book);
    ~Owner();
    void OutputDetails();
    std::string GetName();
private:
    std::string m_name;
    std::shared_ptr<Book> m_book; // Owner hangs onto the book
};

Owner.cpp

#include "stdafx.h"
#include "Owner.h"
#include "Book.h"

Owner::Owner(std::string name, std::shared_ptr<Book> book) : m_name(name), m_book(book) {}

Owner::~Owner() {
    std::cout << "Owner Destroyed" << std::endl;
}

void Owner::OutputDetails() {
    std::cout << m_name << " owns " << std::endl;
    m_book->OutputDetails();
}

std::string Owner::GetName() {
    return m_name;
}

Here is the main.cpp. In this case, book and owner have strong references to each other and will memory leak once _tmain exits its scope. The destructors for both book and owner are not called when I insert breakpoints in the respective destructors.

main.cpp

#include "stdafx.h"
#include <memory>
#include "Book.h"
#include "Owner.h"

int _tmain(int, _TCHAR*)
{
    {
        std::shared_ptr<Book> book = std::shared_ptr<Book>(new Book("Moby Dick"));
        std::shared_ptr<Owner> owner = std::shared_ptr<Owner>(new Owner("George Heriot", book));

        // Introduced a circular dependency so
        // neither gets deleted
        book->SetOwner(owner);

        owner->OutputDetails();
        book->OutputOwnerInformation();
    }

    return 0;
}

I wanted to see if I could reset() the pointers such that the destructor was called and to break the cyclic dependency. According to my understanding of shared_ptr.reset(), the object should become empty.

http://www.cplusplus.com/reference/memory/shared_ptr/reset/

However, my break points in both destructors are not being hit. My assumption would be that because I have reset both book and owner, the reference count would drop to 0 for both and they would be destroyed when _tmain returns.

main2.cpp

#include "stdafx.h"
#include <memory>
#include "Book.h"
#include "Owner.h"

int _tmain(int, _TCHAR*)
{
    {
        std::shared_ptr<Book> book = std::shared_ptr<Book>(new Book("Moby Dick"));
        std::shared_ptr<Owner> owner = std::shared_ptr<Owner>(new Owner("George Heriot", book));

        // Introduced a circular dependency so
        // neither gets deleted
        book->SetOwner(owner);

        owner->OutputDetails();
        book->OutputOwnerInformation();

        owner.reset();
        book.reset();
    }

    return 0;
}

I understand that this is already horrible code and I could use a weak_ptr to remove the cyclic dependency but I am just curious why reset() does not break this dependency.

GECH
  • 83
  • 1
  • 8
  • 3
    But you are not breaking the loop, `book.m_owner` is still pointing to the owner, and `owner.m_book` is still pointing to the book. I think you may be thinking that the shared pointers from main and the ones from your objects are the same, but they are not: they just _point_ to the same objects and share the reference count. – rodrigo Dec 18 '14 at 15:49
  • If you have cyclic ownership, by definition you have a design error. – curiousguy Jul 27 '19 at 21:40

1 Answers1

4

Try printing owner.use_count() and book.use_count() before resetting them. You'll see the use counts are 2. The reset calls will make owner and book decrement their counts by 1, but there are still other shared_ptr objects that share ownership with them and which you don't reset, so the reference counts don't reach zero.

If you think about it you should realise that of course reset() can't break cycles, because the equivalent of reset() happens in the shared_ptr destructor anyway. If the destructor could break cycles like that then there would be no problem creating cycles in the first place.

Jonathan Wakely
  • 166,810
  • 27
  • 341
  • 521
  • Ah, I see. I think I understand what is going on now. In this case, since I have reset both owner and book, are there wild instances of Book and Owner somewhere in memory? – GECH Dec 18 '14 at 16:22
  • No, they're not "wild" the shared_ptr `owner->m_book` shares ownership with `book` and `book->m_owner` shares ownership with `owner`. You have only reset `owner` not `m_book->owner`. When you reset `owner` the refcount goes to 1 and you cannot reset the last shared_ptr because you'd just thrown away the only handle you had to it. – Jonathan Wakely Dec 18 '14 at 17:26
  • Okay, thanks for clearing that. I meant wild in the sense that, I have lost my only handle to `owner` and `book` in _tmain when I do `owner.reset()` and `book.reset()` so I cannot retrieve a handle to `owner` and `book`. – GECH Dec 19 '14 at 12:06