C++ Setting Pointer Equal to shared_ptr

Question

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;
};

At first glance, I'd guess the problem is likely the definition of `Node` — Assimilater, Jul 14 '16 at 02:59
Well it seems like I need the `temp` node to be dynamically allocated or else the program doesn't work, and I was just considering dynamically allocating it with a `shared_ptr` rather than a standard `new`. And I can post the `Node` struct if that will help. — Archie Gertsman, Jul 14 '16 at 03:02
The object pointed to by a `shared_ptr` is destroyed when the last instance of a `shared_ptr` that manages that object is destroyed. In your posted code, this happens at the end of `push_back()` when `temp` goes out of scope. You're then storing dangling pointers in your `head` and `next`. — mkal, Jul 14 '16 at 03:08

score 4 · Answer 1 · answered Jul 14 '16 at 03:08

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.

Thanks for the great, detailed answer. I chose to accept Assimilater's response as his came fist and solved the issue, but I really like your answer :) — Archie Gertsman, Jul 14 '16 at 03:13
@ArchieGertsman I was editing my answer with this kind of info as this answer came up :). It may help you understand a little more of what's going on under the hood — Assimilater, Jul 14 '16 at 03:16

Assimilater · Accepted Answer · 2016-07-14T03:26:39.333

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!

Ahh, I see. I have replaced all occurrences of pointers with smart pointers (not necessarily dynamically allocated like yours), and that his fixed the issue. Thanks. — Archie Gertsman, Jul 14 '16 at 03:10
Actually, sparked by this question, I learned today they can also be implemented as a linked list *themselves* (gotta love the irony). See: http://stackoverflow.com/questions/725142/how-does-a-reference-counting-smart-pointers-reference-counting-work — Assimilater, Jul 14 '16 at 03:46

C++ Setting Pointer Equal to shared_ptr

2 Answers2