How does deletion of dynamic memory really work?

Question

In main I have two types of objects: Person and Pet. A Person can have a number of Pets. I am creating both types of objects dynamically.

#include <iostream>
#include <string>
#include <vector>

struct Pet
{
    Pet() { }

    Pet(std::string name, std::string type)
    {
        Name = name;
        Type = type;
        std::cout << Name << " is created" << std::endl;
    }
    ~Pet()
    {
        std::cout << Name << " is destroyed" << std::endl;
    }

    std::string GetName() { return Name; }
    std::string GetType() { return Type; }

    std::string Name;
    std::string Type;
};

struct Person
{
    Person(std::string name)
    {
        Name = name;
        std::cout << Name << " is created" << std::endl;
    }

    ~Person()
    {
        std::cout << Name << " is destroyed" << std::endl;
    }

    void AddPet(Pet& pet)
    {
        Pets.push_back(pet);
    }

    std::string GetPersonsPets()
    {
        if (Pets.size() == 0)
        {
            return Name + " has no pets";
        }
        if (Pets.size() == 1)
        {
            return Name + " has " + Pets[0].GetName() + ",a " + Pets[0].GetType();
        }
    }

    std::string Name;
    std::vector<Pet> Pets;
};

int main()
{
    Person* peter = new Person("Peter");

    Person* alice = new Person("Alice");

    Pet* fluffy = new Pet("Flyffy", "dog");
    peter->AddPet(*fluffy);

    std::vector<Person*> People;
    People.push_back(peter);
    People.push_back(alice);

    int i = 0;
    for (std::vector<Person*>::iterator it = People.begin(); it != People.end();
        it++)
    {
        std::cout << People[i]->GetPersonsPets() << std::endl;
        i++;
    }
    //delete fluffy;
    delete alice;
    delete peter;
}

Everything seems to work as it should, but something interesting happens when it comes to deleting the Pet objects. When I uncomment delete fluffy, fluffy gets deleted by itself right after peter.

I thought that, since fluffy was created dynamically, it would never be destroyed unless I did that myself?

But the other interesting thing happens when I don't uncomment delete fluffy. Then it's destructor will be called twice.

How is it possible to destroy something twice?

Assuming `Pets` is a `std::vector` (you didn't show), `push_back` creates a copy of the object you give it. The copy is destroyed by the `std::vector`, you are still responsible to destroy the original with `delete`. This, by the way, also makes it pointless to use dynamic memory allocation here. The vector saves it own copy anyway. Please show us what the definitions of your classes are and also try to reduce the code to make it a [repro]. — walnut, Nov 01 '19 at 22:46
Although the previous comment is likely right it's not possible to determine this authoritatively because you failed the requirements for a [mre]. See [ask] for more information. — Sam Varshavchik, Nov 01 '19 at 22:51

463035818_is_not_an_ai · Accepted Answer · 2019-11-01T23:04:12.080

When you add the pet here:

peter->AddPet(*fluffy);

then *fluffy is the pointer derferenced and here

void Person::AddPet(Pet& pet)
{
Pets.push_back(pet);
}

You store a copy of *fluffy in some container (I suppose it is a std::vector).

The objects you see getting destroyed are those copies (vector manages its own memory, ie it destroys its elements when the vector is destroyed). You still need to delete the dynamically allocated instances you created in main.

Note that you better didnt use new and delete at all here. Use dynamic memory when you must. And when you do you better use smart pointers rather than raw pointers.

PS Your code exhibits the bad practice of writing getters just because. All the members of Pet and Person are public. Either make the members private or remove the getters. Having both is misleading. Also if you do provide a default constuctor ( Pet() { }) it should initialize the members to meaningful values. The job of a constructor is to construct an object in a valid state. Last but not least you should use the member initializer list for constructors as in

Pet(std::string name, std::string type) : Name(name),Type(type) {}

Your edits are referring to code not written by OP, but rather by an edit from someone else in an attempt to make an MCVE without OP's input. I am not sure whether this is acceptable, but if it is, rather than answering on it, I would suggest editing the question to remove unrelated problems introduced by edits. — walnut, Nov 01 '19 at 22:59
@uneven_mark its only the public member vs getters part that was not present in the orginial post. Anyhow, my answer applies to the question as it stands — 463035818_is_not_an_ai, Nov 01 '19 at 23:02

score 3 · Answer 2 · answered Nov 01 '19 at 22:48

Pets.push_back(pet);

creates a copy of pet and stores it inside vector. When vector is destroyed, this copy is destroyed (but the original fluffy allocated with new is still leaked).

When you uncomment delete fluffy;, then you properly destroy both the original object and it's copy.

If you want to track copies of objects created, add user defined copy constructor and copy assignment operator (read up on The Rule of Three).

score 0 · Answer 3 · answered Nov 01 '19 at 23:16

Remember that what goes into a container is the copy of the object you specify. In your case, copy of the object *fluffy goes into the container. It is this copy that gets destroyed (thus calling its destructor) and not the original newed object created in this line Pet* fluffy = new Pet("Flyffy", "dog");. Thus, your code with //delete fluffy; results into a memory leak.

In case you consider copying the pointer itself into the container, note that when using containers of newed pointers, remember to delete the pointers before the container is destroyed (Item 7, Effective STL, Scott Meyers.) since destructing a pointer object does not call delete. Or else use smart_pointers as mentioned by @formerlyknownas_463035818

How does deletion of dynamic memory really work?

3 Answers3