CODEWITHSUNDEEP
c++memory-managementsmart-pointersunique-ptr
ra1nmaster
ra1nmaster

Reputation: 702

Is this behavior of `std::unique_ptr` expected?

I have a project that I've been working on that uses polymorphism to implement the encoding of objects via overriding an encode function.

Now, this project has been using pointers to implement the polymorphic behavior. Since my application was relatively new, I decided that it wouldn't be too difficult to switch from regular raw pointers into smart pointers, as they would automatically manage memory.

In one of my functions, I return an std::unique_ptr. After returning the smart pointer, I immediately wanted to cast the underlying raw pointer. So an oversimplification of what was happening would look something like this:

std::unique_ptr<something> do_something() {
    return std::unique_ptr<something>(new something());
}

void caller() {
    auto output = static_cast<another*>(do_something().get());
    // Do something with output...
}

I knew that std::unique_ptr doesn't have a copy constructor, as it can only be "owned" once at any given instant. Implementing a copy constructor would allow for the creation of a duplicate pointer, going against this principle. If my understanding is correct, the following code initializes B using the move constructor, and does not copy A:

std::unique_ptr<int> get_a() {
    std::unique_ptr<int> A(new int);
    return A;
}

void get_b() {
   auto B = get_a();
}

This was exactly the behavior that I wanted. However, in my example, the program crashes around half of the time. The other half of the time, it appears to work, but the contents of the underlying pointer are just plain garbage. (I'm suspecting that this was undefined behavior, and get() was only sometimes returning a readable address.)

Now, after some thought, I came to the conclusion that if B was indeed being initialized via a move constructor, B itself would have to not be a temporary. However, in my first example, the result of do_something() is a temporary, and I'm move constructing it! So I decided to stop inlining the call to do_something(), and then I immediately got the expected behavior! Every time I used get() on a temporary object, the results were unpredictable and simply wrong.

My question is:

Should I be expecting this behavior of unique_ptr? If so, how is this behavior documented? (More specifically, was my first example undefined behavior?) And are there any alternatives (shared_ptr, maybe?) that would allow me to inline the function call and still get the expected results?

Upvotes: 0

Views: 318

Answers (1)

Lightness Races in Orbit
Lightness Races in Orbit

Reputation: 385395

The key here is summarised in your statement "they would automatically manage memory".

Your unique_ptr (post-move) is a temporary that goes out of scope immediately after the declaration of output, taking the dynamically-allocated int with it and leaving output dangling.

Temporary. Temporary! So-named for good reason…

This behaviour is "documented" in many, many places: storing a pointer to an object that has gone out of scope is a really bad idea, and henceforth using that pointer is just outright broken (yes, it's UB).

You can't hack around this by switching to other smart pointers, no: stop "inlining" the function call (and stop using this terminology; that's not what "inlining" means) and ensure that the object exists for as long as you want to access it through a pointer.

auto ptr    = do_something();
auto output = static_cast<another*>(ptr.get());

Upvotes: 2

Related Questions