CODEWITHSUNDEEP
c++
Dave
Dave

Reputation: 81

Destructor being called early

I have a queue class wherein I'm trying to dynamically allocate 3 "sample" objects and place them into a queue, and then dequeue and delete them. But the destructor for the sample objects:

~Sample() { cout << "Destructing Sample object " << id << "\n"; }

Is for some reason being called when I try to put the objects on the queue.

int main()
{
   Sample *samp1;
   Sample *samp2;
   Sample *samp3;
   Queue<Sample> sQa(3);
   samp1 = new Sample(1);
   samp2 = new Sample(2);
   samp3 = new Sample(3);

   cout << "Adding 3 Sample objects to queue...\n";

   sQa.put(*samp1);
   sQa.put(*samp2);
   sQa.put(*samp3);

   cout << "Dequeing and destroying the objects...\n";

   for(int i = 0; i < 3; ++i)
   {
       delete &sQa.get();
   }

   return 0;
}

Here is the output:

Adding 3 sample objects to queue...
Destructing Sample object 1
Destructing Sample object 2
Destructing Sample object 3
Dequeing and destroying the objects...
Destructing Sample object 1

And then the program crashes. Anyone know why this might be? Also, here is the put function in case it is necessary. (the queue class is a template)

void put(Qtype data) 
{
    if(putloc == size) 
    {
        cout << " -- Queue is full.\n";
        return;
    }

    putloc++;
    q[putloc] = data;   
}

Upvotes: 2

Views: 754

Answers (4)

Mike Seymour
Mike Seymour

Reputation: 254411

You code allocates objects; copies them into the queue; retrieves copies from the queue; and tries to delete those copies. The original objects are leaked (since you don't delete samp1 etc.), and the final objects can't be deleted since they weren't created with new.

The destructors you're seeing are from temporary objects created while you copy the allocated objects into the queue; the crash is the undefined behaviour you get from deleting the wrong object.

Since the queue contains objects, not pointers, there is no need to mess around with dynamic allocation at all:

cout << "Adding 3 Sample objects to queue...\n";

sQa.put(Sample(1));
sQa.put(Sample(2));
sQa.put(Sample(3));

cout << "Dequeing and destroying the objects...\n";

for(int i = 0; i < 3; ++i)
{
   sQa.get();
}

If you want to experiment with dynamic allocation, then you should modify the queue to store pointers; but remember that it's very difficult to manage dynamic objects without the help of RAII so don't try this in any code that needs to be reliable.

Upvotes: 1

masoud
masoud

Reputation: 56479

The root of your problem

delete &sQa.get();

You're deleting something which is irrelevant to the allocated objects by new.

The correct way is something like below:

Queue<Sample*> sQa(3);
samp1 = new Sample(1);
samp2 = new Sample(2);
samp3 = new Sample(3);

sQa.put(samp1);
sQa.put(samp2);
sQa.put(samp3);

cout << "Dequeing and destroying the objects...\n";

for(int i = 0; i < 3; ++i)
{
   delete sQa.get();
}

Note: it's better to use smart pointers such as unique_ptr or shared_ptr instead bare pointers.

Upvotes: 3

Luis
Luis

Reputation: 1235

Unless Qtype is a typedef, your put() function specifies its parameter should be passed by copy. Whenever you call it, it will call Sample's copy constructor to create a temporary object.

Upvotes: 1

Alok Save
Alok Save

Reputation: 206498

Your code causes Undefined behavior. It calls delete on a object which was not dynamically allocated using new.

With: 

sQa.put(*samp1);

You store a copy of the dynamically allocated member in sQa. You lost the pointer to dynamically allocated objects once you copied the object by value.

The solution is to use smart pointer as the container element and let RAII do all the hard work of memory management for you.

Upvotes: 10

Related Questions