16

My question concerns shared_ptr and make_shared in C++11. I have two vectors, the first one stores smart pointers and the second one stores raw pointers. The first vector works as I had expepted but vector2 is just confusing...

Code sample

#include <iostream>
#include <vector>
#include <memory>

int main() {
    std::vector<std::shared_ptr<int>> vector1;
    vector1.push_back(std::make_shared<int>(1));
    vector1.push_back(std::make_shared<int>(2));
    vector1.push_back(std::make_shared<int>(3));

    std::vector<int*> vector2;
    vector2.push_back(std::make_shared<int>(4).get());
    vector2.push_back(std::make_shared<int>(5).get());
    vector2.push_back(std::make_shared<int>(6).get());

    std::cout << "vector1 values:" << std::endl;
    for(auto &value: vector1) { std::cout << *value << std::endl; }

    std::cout << "vector2 values:" << std::endl;
    for(auto &value: vector2) { std::cout << *value << std::endl; }

    return 0;
}


Output

vector1 values:
1
2
3
vector2 values:
6
6
6


Question

I realize that it would be much simpler to create raw pointers to begin with and not try to convert smart pointers but it made me curious to know why this is happening? Also why does each push change all of the values in vector2?


Links

Here are some questions that I found in stackoverflow but they didn't answer my question or maybe I didn't understand the answers...

Community
  • 1
  • 1

2 Answers2

10

You are seeing undefined behaviour. When you do this:

vector2.push_back(std::make_shared<int>(4).get());

You are creating a temporary shared_ptr, and copying a pointer to its managed object into your vector. This immediately becomes a dangling pointer.

juanchopanza
  • 223,364
  • 34
  • 402
  • 480
10

The reason you'll use a shared_ptr is that you want the memory it points to to be freed when all instances that point to it go out of scope. The shared_ptr is destroyed right after you call .get() on it, so you immediately have a dangling pointer. The result of the dereference operation is undefined, which means that it may or may not return a value that makes sense, or it could even do something completely unrelated (like crashing).

This is a feature. You want this to happen: otherwise, you'd be leaking memory. Imagine this code:

vector<int> integers;
integers.push_back(*make_shared<int>(6).get());

If the memory wasn't freed, there would be no way to release it afterwards, because there's no way you could recover the shared_ptr's managed pointer.

zneak
  • 134,922
  • 42
  • 253
  • 328
  • 1
    This makes a little bit more sense now, but wouldn't it make more sense if `std::shared_ptr::get` returned an entirely new pointer instead of a pointer to the managed object? –  May 07 '14 at 22:12
  • 2
    @Oak, this would force any type that you use with a `shared_ptr` to be copiable, and it would very possibly not work with polymorphism because of C++'s copy mechanism. It would also be very slow on large data structures, and it would also burden the developer with freeing the pointer (which goes against the point of using a `shared_ptr` in the first place). You also couldn't use it with array types because it's impossible to get the length of an array created with `new` (and therefore the `shared_ptr` wouldn't know how much data to copy when returning a new pointer). – zneak May 07 '14 at 22:18
  • Ah, thank you very much for an elaborate and easily understandable answer :) –  May 07 '14 at 22:21
  • 1
    @Oak No, because the `get()` method is used to extract the pointer out of the smart pointer, not to get a completely new pointer. You're probably getting a '6' as the output since each of your three allocations are probably getting the same piece of memory, thus you're only getting the last value. However (as mentioned elsewhere) this is undefined behaviour as you are dereferencing a pointer which has already been deleted. – Andre Kostur May 08 '14 at 01:35