shared_ptrs being deleted twice

Question

I want to store shared pointers to the Object class in a vector:

Test code:

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

using namespace std;   // only for brevity

class Object
{
public:
  int n;
  Object(int n) : n(n) { cout << "Object("  << n <<")\n"; }
  ~Object() { cout << "~Object(" << n << "))\n"; n = 0xdddddddd; }

};

void Test()
{
  std::shared_ptr<Object> p1(make_shared<Object>(Object(123)));
  std::vector<shared_ptr<Object>> v;

  cout << "before push_back\n";
  v.push_back(std::make_shared<Object>(Object(2)));
  v.push_back(p1);
  cout << "after push_back\n";

  cout << "Vector content:\n";
  for (auto& p : v)
    cout << "  " << p->n << "\n"; ;
}

int main()
{
  Test();
  cout << "after Test()\n";
}

The output is

Object(123)
~Object(123))        <<< why is the destructor called here?
before push_back
Object(2)
~Object(2))          <<< why is the destructor called here?
after push_back
Vector content:
  2
  123
~Object(2))          <<< destructor called again 
~Object(123))
after Test()

I don't understand why the destructors are called twice.

OTOH the vector content is what I want.

`Object(123)` and `Object(2)` creates *temporary* objects that will be destructed. — Some programmer dude, Feb 20 '20 at 14:41
You should print `this` in your `cout`'s to get a better view of what is deleted. — PaulMcKenzie, Feb 20 '20 at 14:43
@Someprogrammerdude but why do I see _one_ contructor call and _two_ desctructor calls for each object? Shouldn't be there _two_ constructor calls? — Jabberwocky, Feb 20 '20 at 14:44
Because the temporary objects will be *copied* (or moved) using the copy-constructor (or move-constructor). You don't implement those. And even if you implement them, the compiler can [*elide*](https://en.cppreference.com/w/cpp/language/copy_elision) (not call) them. — Some programmer dude, Feb 20 '20 at 14:48
When logging constructor/destructor, don't forget the copy and move ones. — Jarod42, Feb 20 '20 at 14:53
Duplicate? [Why is the destructor of the class called twice?](https://stackoverflow.com/questions/2627540/why-is-the-destructor-of-the-class-called-twice) — Yksisarvinen, Feb 20 '20 at 15:09

NutCracker · Accepted Answer · 2020-02-20T21:14:13.203

I don't understand why the destructors are called twice.

Because of creating unnecessary temporary objects here:

std::shared_ptr<Object> p1(make_shared<Object>(Object(123)));
                                               ^^^
                                               temporary object

and here:

v.push_back(std::make_shared<Object>(Object(2)));
                                     ^^^
                                     temporary object

It should instead be

std::shared_ptr<Object> p1(make_shared<Object>(123));

and

v.push_back(std::make_shared<Object>(2));

Why?

Because std::make_shared constructs an object of type T and wraps it in a std::shared_ptr using args as the parameter list for the constructor of T. And in your code, you are making one extra object which immediately gets destroyed, thus calling the destructor.

Why don't you see Object(int n); constructor being called for temporary object?

Object(int n); constructor is indeed being called for the temporary object, but since the object held by the std::shared_ptr is created through copy constructor (so, by copying the temporary object) you won't see call to Object(int n); for it but call to Object(Object const& other);.

In the demo, you can see first Object(int n); constructor being called for the temporary object and then the call to copy constructor Object(Object const& other); for the actual object being referenced by the std::shared_ptr.

_" And in your code, you are making one extra object which immediately gets destroyed, thus calling the destructor"_: OK, I get this, but why don't I see the __construction__ of the temporary object? — Jabberwocky, Feb 20 '20 at 14:48
@Jabberwocky because copy constructor is being called. Check [here](https://godbolt.org/z/CHXkff) — NutCracker, Feb 20 '20 at 14:49
@Jabberwocky I have updated my answer. Let me know if you have additional questions — NutCracker, Feb 20 '20 at 14:57
@Jabberwocky, you *do* see the construction of the temporary. What you're not seeing is the copy-construction of the shared object. — Toby Speight, Feb 20 '20 at 15:03

score 5 · Answer 2 · answered Feb 20 '20 at 14:41

This is because you have to destroy the temporary values.

The std::make_shared function takes any amount of parameter and construct a value of the given type with it.

You construct a Object and pass it to std::make_shared, which in its turn construct a value using new. Then, the temporaries are destroyed. Later, the shared pointers also are destroyed.

Simply change this in your code:

std::shared_ptr<Object> p1(make_shared<Object>(123));

// ...  

v.push_back(std::make_shared<Object>(2));

And you'll see only one destructor for each values.

shared_ptrs being deleted twice

2 Answers2