CODEWITHSUNDEEP
c++c++11
Archie Gertsman
Archie Gertsman

Reputation: 1661

C++ Setting Pointer Equal to shared_ptr

I'm trying to make a List class using the concept of linked lists, and while I was originally using the C++ standard new keyword, I decided to switch it out for the C++11 std::shared_ptr. However, I can't get the program to function properly when using smart pointers, as it crashes. Here are some bits of code before the change:

class List
{
public:
    void push_back(...) {
        Node *temp = new Node;
        ...
        if (!head) {
            head = temp;
            return;
        }
        else {
            Node *last = head;
            ...
            last->next = temp;
        }
    }
    ...

private:
    Node *head = nullptr;
};

And here's what it looks like with the change:

class List
{
public:
    void push_back(...) {
        std::shared_ptr<Node> temp(new Node);
        ...
        if (!head) {
            head = temp.get();
            return;
        }
        else {
            Node *last = head;
            ...
            last->next = temp.get();
        }
    }
    ...

private:
    Node *head = nullptr; // don't need this to be a smart ptr
};

I feel like the problem might be that head and last aren't dynamically allocated and maybe they need to be to work with a shared_ptr, but I'm not sure. What exactly am I doing wrong and how can I fix it? I really hope this isn't a duplicate because I can't seem to find anything that solves my problem. Thanks.

Edit: Here's the Node struct:

struct Node{
    int data;
    Node* next;
};

Upvotes: 1

Views: 1484

Answers (2)

Assimilater
Assimilater

Reputation: 964

Your main issue is that if you're going to use shared_ptr, best to use it all the way. Make next a shared_ptr instead of a raw one.

struct Node {
    int data;
    std::shared_ptr<Node> next;
}

What std::shared_ptr does under the hood is keep a count of how many references there are to a pointer. When you use copy constructors or operator= it increases a reference count. When an instance falls out of scope resulting in the destructor being invoked (or you give it a different pointer with operator=) the reference count decrements. When the count is zero the pointer is destroyed.

// pass by value invokes copy constructor (refcount + 1)
void myFunc(std::shared_ptr<MyClass> var) {

    // Code using var

} // end of function invokes destructor (refcount - 1)

void run() {
    std::shared_ptr<MyClass> ptr(new MyClass); // refcount = 1
    myFunc(ptr); // refcount = 2
    // After myFunc returns refcount = 1

}
int main() {
    run(); // refcount = 1
    // After run returns, refcount = 0 and the pointer is deleted
}

By using get() you introduce a pointer to memory that may be deleted at some point, regardless of whether or not that pointer is around. This can lead to segfaults as the raw pointers are pointing to memory that shared_ptr deleted.

This is because get() does not affect the reference count. How could it? It's not a shared_ptr any more so that class definition has no way of knowing what you do with it, or when it gets deleted. If get() increased the reference count there would be nothing to decrease it afterwards, and the memory would never be released. That's a memory leak!

Upvotes: 1

Sam Varshavchik
Sam Varshavchik

Reputation: 118300

The reason to have std::shared_ptr in the first place is to have std::shared_ptr take complete and full ownership of the pointer, and make it std::shared_ptr's responsibility to delete it, once the last reference to the pointer goes away. That's what std::shared_ptr is all about.

This means that once a pointer is placed into a std::shared_ptr, the std::shared_ptr now takes complete and full responsibility of managing the pointer. It owns it completely.

It, therefore, makes no sense to put a pointer into a std::shared_ptr ... and then immediately take it out:

head = temp.get();

There are reasons for the get() function to exist, but this isn't one of them.

In order to use std::shared_ptr correctly, everything must be a std::shared_ptr. head needs to be a std::shared_ptr:

std::shared_ptr<Node> head; // yes, it does need to be a smart ptr

Why does it need to be a std::shared_ptr? Well, if it's not, what do you think will happen when this:

std::shared_ptr<Node> temp(new Node);

Specifically, when this temp smart pointer gets destroyed, when this function returns? Well, since it will be the last std::shared_ptr that referenced this Node, it will happily delete it. The fact that you get() it earlier, and placed it into head doesn't matter. So now you have a head that points to a deleted Node. Hilarity ensues.

And this is why everything must be a std::shared_ptr. Not only head, but also Node's next member also needs to be a std::shared_ptr, too.

Now, there is a pitfall involving circular references, that comes into play when std::shared_ptr enters the picture. But that's going to be a different question.

Upvotes: 4

Related Questions