1

Shouldn't std::unique_ptr prevent the possibility of such an error?

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

struct B {
    int b;
};

int main()
{
    std::vector<std::unique_ptr<B>> v;  // unique_ptr can be stored in a container
    B* p = new B;
    v.emplace_back(p);
    std::cout << "p:" <<p <<"\n";
    std::cout << "v[0]:"<<v[0].get() << "\n";
    v.emplace_back(p);
    std::cout << "p:" <<p <<"\n";
    std::cout << "v[1]:"<<v[1].get() << "\n";

}

Error message when double free is detected:

*** Error in `./a.out': double free or corruption (fasttop): 0x0000000001094c20 ***
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(+0x777e5)[0x7f3da200b7e5]
...
======= Memory map: ========
00400000-00402000 r-xp 00000000 ca:01 3228321                            /tmp/1617748074.6996937/a.out
...

Output when double free is ignored:

p:0x1094c20
v[0]:0x1094c20
p:0x1094c20
v[1]:0x1094c20
Jarod42
  • 173,454
  • 13
  • 146
  • 250
K.Karamazen
  • 152
  • 1
  • 8
  • 4
    You're only supposed to put a specific single pointer into a unique_ptr _once_. When you do it thinks it owns it, and will free it. It doesn't know you've already put it into some _other_ unique_ptr. You've got to not do that. (Think about how it could possibly prevent the situation you're describing: Each unique_ptr has to know about _all other_ unique_ptrs you've got to make sure its the only one holding what it's holding? No. It's on you to use it correctly.) – davidbak Apr 06 '21 at 22:46
  • 2
    It should work like this. You construct two unique pointer from the same raw pointer, so both tries to deallocate it at the end – Iter Ator Apr 06 '21 at 22:47
  • 1
    Remember that a pointer is one-way only. There is no mechanism for a pointer to know who else is pointing at an object `shared_ptr` doesn't even know who else is pointing. It only knows the count. – user4581301 Apr 06 '21 at 22:52
  • 3
    Suggestion: Avoid this possibility by religiously using [`std::make_unique`](https://en.cppreference.com/w/cpp/memory/unique_ptr/make_unique). – user4581301 Apr 06 '21 at 22:53
  • @davidbak not just all other `unique_ptr`s, all `B`s (that might have automatic or static storage duration) – Caleth Apr 06 '21 at 22:56
  • 2
    C++ does not stop you from doing incorrect things – Caleth Apr 06 '21 at 22:58
  • By what algorithm should the `vector>` prevent this error? – Howard Hinnant Apr 07 '21 at 00:03
  • Howard Hinnant, lately I have been studying unique_ptr a lot. I would never use it this way. But somehow I would have expected it to only allow constructing from rvalue. That is clearly not the case. – K.Karamazen Apr 07 '21 at 13:49
  • `unique_ptr` doesn't construct from an lvalue or an rvalue, it constructs from a pointer. – Marshall Clow Apr 07 '21 at 16:06
  • @MarshallClow, hm, I think you know this matter better than me, but between `B* p = new B; std::unique_ptr x(p);` and `std::unique_ptr x(new B);`, the first one is constructing from lvalue and the second one from rvalue, right? – K.Karamazen Apr 07 '21 at 16:38
  • `p` is an lvalue, but as you've seen `unique_ptr` doesn't care. All it wants is the object that `p` points at and takes a pointer by value (remember that a pointer is just another variable and can be passed by value or reference depending on the needs of the function). For details see constructor overloads 3 and 4 [at this documentation page](https://en.cppreference.com/w/cpp/memory/unique_ptr/unique_ptr). Most of the really gory details have to do with how the optional deleter is passed. – user4581301 Apr 07 '21 at 17:04
  • Side note: In case you haven't been exposed to how much fun rvalues can get, give [What are rvalues, lvalues, xvalues, glvalues, and prvalues?](https://stackoverflow.com/questions/3601602/what-are-rvalues-lvalues-xvalues-glvalues-and-prvalues) a read. – user4581301 Apr 07 '21 at 17:05
  • Supposing that `std::unique_ptr` (could and) did “prevent the possibility of such an error”—what would that look like? Would the program not compile, and if so based on what static(!) rule? Would it raise an exception, and if so at what runtime cost? – Davis Herring Apr 08 '21 at 02:56

2 Answers2

6

std::unique_ptr can prevent double free bugs if used properly:

  • Never create an owning raw pointer; use std::make_unique.
  • If you must absolutely create an owning raw pointer using new (i.e. in order to call a private constructor from a factory function), then do so within a statement that immediately wraps the raw pointer in a std::unique_ptr and does nothing else, e.g. return std::unique_ptr<Base>(new Derived(...));
  • Do not use the std::unique_ptr constructor that takes a raw pointer in any other context.

If these guidelines are followed, each unique_ptr will manage an object that is not managed by anyone else. If they are not followed, there is a possibility of bugs like the one you observed, where multiple unique_ptr instances can be created from the same raw pointer. Neither instance is "aware" that the raw pointer you pass to it is already owned by another instance. If the unique_ptr constructor had to check whether anyone else owns the pointer, it would introduce performance overhead for every single user. We use C++ to avoid such unnecessary overhead. It is unnecessary as long as you follow the guidelines.

Brian Bi
  • 91,815
  • 8
  • 136
  • 249
4

Minimal example (no need for vector):

struct B {
    int b;
};

int main()
{
    B* p = new B;
    std::unique_ptr<B> x(p);
    std::unique_ptr<B> y(p);
}

You have two unique_ptrs who believe that they own the same raw pointer. They're both going to call delete on that pointer, and you get a double free. Putting the unique_ptrs in a container doesn't change that.

Marshall Clow
  • 13,643
  • 1
  • 24
  • 44
  • Nice simplification. I have submitted a change from `x(b)` to `x(p)`. In the original wrong version you have `std::unique_ptr x(b);` what is wrong. However, it must be approved first. – K.Karamazen Apr 08 '21 at 09:54