CODEWITHSUNDEEP
c++pointersshared-ptr
simplename
simplename

Reputation: 919

object containing smart pointer to itself that isn't reset before object goes out of scope

Ran into this bug where a class contains a pointer to itself -- and it was pointing to the same instance. Here is the smallest code snippet that contains that problem... 

class Node
{
public:
    Node()
    {
        std::cout << "constructor" << std::endl;
    }

    ~Node()
    {
        std::cout << "destructor" << std::endl;
    }

    void storeChild(boost::shared_ptr<Node> ptr)
    {
        m_ptr = ptr;
    }

private: 
    boost::shared_ptr<Node> m_ptr;
};

int main()
{
    boost::shared_ptr<Node> bc_ptr(new Node());
    bc_ptr->storeChild(bc_ptr); // this line is the bug 
    return 0;
}

My questions are: 1. Is the Node object ever deleted? I know smart pointers are supposed to manage those things so we don't have to delete. But it looks like the reference that's passed into the class using storeChild never is reset. Does this mean I'm going to have a memory leak? 2. Is there a way to use smart pointers that prevents this from happening? Clearly the storeChild method should be given a pointer to a different Node but how do you prevent that?

If I run this program, the destructor never gets called.

Upvotes: 0

Views: 871

Answers (1)

Ted Lyngmo
Ted Lyngmo

Reputation: 117298

Yes, this is a memory leak since the shared pointer in Node never will be deleted. Consider:

#include <iostream>
#include <memory>

class Node {
    std::shared_ptr<Node> m_ptr;

public:
    Node() { std::cout << "constructor\n"; }
    ~Node() { std::cout << "destructor\n"; }
    void storeChild(std::shared_ptr<Node> ptr) { m_ptr = ptr; }
    long use_count() { return m_ptr.use_count(); }
};

int main() {   
    Node* n = new Node;
    {
        std::shared_ptr<Node> bc_ptr(n);
        bc_ptr->storeChild(bc_ptr); // this line is the bug

        std::cout << n->use_count() << "\n";     // prints 2
    }
    std::cout << n->use_count() << "\n";         // prints 1

    // the Node pointed to by n is leaked
}

It's probably even more obvious if you do like this:

#include <iostream>
#include <memory>

class Node {
    std::shared_ptr<Node> m_ptr;

public:
    Node() : m_ptr(this) { std::cout << "constructor\n"; }
    ~Node() { std::cout << "destructor\n"; }
};

int main() {
    new Node;
}

If you try:

int main() {
    delete new Node;
}

The Node destructor will be called twice. First by your delete and secondly by the shared_ptr when it's deleted.

Preventing it can be tricky if it's possible to end up with circular ownership, like: A->B->C->A ... You should probably use std::weak_ptr if that's a risk.

Upvotes: 1

Related Questions