destroying std::queue leads to memory error

Question

I have produced a minimal code in C++ leading to a memory error I do not understand.

#include <queue>

using namespace std;

class A{
    int *a;
public:
    A() {a=new int[2];}
    ~A() {delete[] a;}
};

main() {
    queue<A> *q=new queue<A>;
    q->push(A());
    //q->pop();
    delete q;
}

This leads to

*** Error in `./a.out': double free or corruption (fasttop): 0x000055b703ee3f30 ***

I have no idea why. It also yields this error if I uncomment the q->pop() line, so I have an empty queue.

Valgrind says

==17708== Invalid free() / delete / delete[] / realloc()
==17708==    at 0x4C2D7DB: operator delete[](void*) (vg_replace_malloc.c:621)
==17708==    by 0x108C2A: A::~A() (pokus.cpp:9)
==17708==    by 0x10A003: void std::_Destroy<A>(A*) (stl_construct.h:93)
==17708==    by 0x109BD0: void std::_Destroy_aux<false>::__destroy<A*>(A*, A*) (stl_construct.h:103)
==17708==    by 0x109979: void std::_Destroy<A*>(A*, A*) (stl_construct.h:126)
==17708==    by 0x1096EB: void std::_Destroy<A*, A>(A*, A*, std::allocator<A>&) (stl_construct.h:151)
==17708==    by 0x109372: std::deque<A, std::allocator<A> >::_M_destroy_data_aux(std::_Deque_iterator<A, A&, A*>, std::_Deque_iterator<A, A&, A*>) (deque.tcc:845)
==17708==    by 0x108F32: std::deque<A, std::allocator<A> >::_M_destroy_data(std::_Deque_iterator<A, A&, A*>, std::_Deque_iterator<A, A&, A*>, std::allocator<A> const&) (stl_deque.h:2035)
==17708==    by 0x108CBE: std::deque<A, std::allocator<A> >::~deque() (stl_deque.h:1041)
==17708==    by 0x108C45: std::queue<A, std::deque<A, std::allocator<A> > >::~queue() (stl_queue.h:96)
==17708==    by 0x108B3E: main (pokus.cpp:16)
==17708==  Address 0x5a86290 is 0 bytes inside a block of size 8 free'd
==17708==    at 0x4C2D7DB: operator delete[](void*) (vg_replace_malloc.c:621)
==17708==    by 0x108C2A: A::~A() (pokus.cpp:9)
==17708==    by 0x108B2D: main (pokus.cpp:14)
==17708==  Block was alloc'd at
==17708==    at 0x4C2C93F: operator new[](unsigned long) (vg_replace_malloc.c:423)
==17708==    by 0x108BF5: A::A() (pokus.cpp:8)
==17708==    by 0x108B0E: main (pokus.cpp:14)

If I uncomment the q->pop(), I get

==17974== Invalid free() / delete / delete[] / realloc()
==17974==    at 0x4C2D7DB: operator delete[](void*) (vg_replace_malloc.c:621)
==17974==    by 0x108C36: A::~A() (pokus.cpp:9)
==17974==    by 0x1099A1: void __gnu_cxx::new_allocator<A>::destroy<A>(A*) (new_allocator.h:124)
==17974==    by 0x10950C: void std::allocator_traits<std::allocator<A> >::destroy<A>(std::allocator<A>&, A*) (alloc_traits.h:487)
==17974==    by 0x108FFD: std::deque<A, std::allocator<A> >::pop_front() (stl_deque.h:1554)
==17974==    by 0x108D77: std::queue<A, std::deque<A, std::allocator<A> > >::pop() (stl_queue.h:271)
==17974==    by 0x108B39: main (pokus.cpp:15)
==17974==  Address 0x5a86290 is 0 bytes inside a block of size 8 free'd
==17974==    at 0x4C2D7DB: operator delete[](void*) (vg_replace_malloc.c:621)
==17974==    by 0x108C36: A::~A() (pokus.cpp:9)
==17974==    by 0x108B2D: main (pokus.cpp:14)
==17974==  Block was alloc'd at
==17974==    at 0x4C2C93F: operator new[](unsigned long) (vg_replace_malloc.c:423)
==17974==    by 0x108C01: A::A() (pokus.cpp:8)
==17974==    by 0x108B0E: main (pokus.cpp:14)

For some reason it calls the destructor of some element of the class A even though the queue is empty.

Possible duplicate of [Rule-of-Three becomes Rule-of-Five with C++11?](https://stackoverflow.com/questions/4782757/rule-of-three-becomes-rule-of-five-with-c11) — 1201ProgramAlarm, Aug 11 '17 at 18:13
If not observing Rule of Five, then certainly watch out for the [Rule of Three.](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three). TL;DR: if you need a destructor then you almost always need a copy constructor and operator =. — user4581301, Aug 11 '17 at 18:36

Cory Kramer · Accepted Answer · 2017-08-11T18:15:29.587

4

Your destructor is wrong

~A() {delete a;}

should be

~A() {delete[] a;}

You must always match new/delete and new[]/delete[]

Also you should either make your class non-copyable, or define a user-defined copy constructor which allocates a new array for the copy.

In fact, I'd suggest you change A to the following

#include <array>

class A{
    std::array<int, 2> a;
public:
    A() = default;
};

edited Aug 11 '17 at 18:15

answered Aug 11 '17 at 18:12

Cory Kramer

114,268
16
167
218

Oh, sorry, sure, I was trying to write it quickly. However, the problem is still there, I will edit the question. – Daniel Aug 11 '17 at 18:15
Before +1, you should probably be specific about why the copy constructor is necessary. – Fantastic Mr Fox Aug 11 '17 at 18:15
Oh, the copy constructor, sure. I didn't realize that although I do not need it, some standard function could need it. Thanks. – Daniel Aug 11 '17 at 18:36

destroying std::queue leads to memory error

1 Answers1