Memory error while inserting an object into a vector

Question

I have written a simple class:

class A
{
  private:
    // a bunch of attributes
    int *a;
  public:
    A()  { 
        cout << "constructor called" << endl;
    }
    ~A() { 
        cout << "destructor called" << endl;
        if (a) delete[] a;
    }
    void initialize(int i) {
        //does the initialization
        a = new int[i];
    }
};

and in my program I try to build a vector of these objects but I get double free or corruption error:

int main()
{
    vector<A> a;
    for (int i=0; i<10; i++) {
        a.resize(a.size()+1);
        a[a.size()-1].initialize(i);
    }
}

In fact, when I run this program, I am expecting to get 10 message constructor called and 10 message destructor called after that but instead I get this output:

constructor called
destructor called
constructor called
destructor called
destructor called
constructor called
destructor called
*** glibc detected *** ./test.o: double free or corruption (fasttop): 0x00000000022bc030 ***
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(+0x7eb96)[0x7f823cd49b96]
./test.o[0x400d3b]
./test.o[0x401924]
./test.o[0x401672]
./test.o[0x401107]
./test.o[0x400fb5]
./test.o[0x4014d5]
./test.o[0x401012]
./test.o[0x400e79]
./test.o[0x400bd1]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xed)[0x7f823ccec76d]
./test.o[0x400ac9]
======= Memory map: ========
00400000-00403000 r-xp 00000000 00:20 286720734                          /home/users/u5410055/Desktop/test.o
00602000-00603000 r--p 00002000 00:20 286720734                          /home/users/u5410055/Desktop/test.o
00603000-00604000 rw-p 00003000 00:20 286720734                          /home/users/u5410055/Desktop/test.o
022bc000-022dd000 rw-p 00000000 00:00 0                                  [heap]
7f823c9cf000-7f823caca000 r-xp 00000000 08:03 721005                     /lib/x86_64-linux-gnu/libm-2.15.so
7f823caca000-7f823ccc9000 ---p 000fb000 08:03 721005                     /lib/x86_64-linux-gnu/libm-2.15.so
7f823ccc9000-7f823ccca000 r--p 000fa000 08:03 721005                     /lib/x86_64-linux-gnu/libm-2.15.so
7f823ccca000-7f823cccb000 rw-p 000fb000 08:03 721005                     /lib/x86_64-linux-gnu/libm-2.15.so
7f823cccb000-7f823ce80000 r-xp 00000000 08:03 720994                     /lib/x86_64-linux-gnu/libc-2.15.so
7f823ce80000-7f823d080000 ---p 001b5000 08:03 720994                     /lib/x86_64-linux-gnu/libc-2.15.so
7f823d080000-7f823d084000 r--p 001b5000 08:03 720994                     /lib/x86_64-linux-gnu/libc-2.15.so
7f823d084000-7f823d086000 rw-p 001b9000 08:03 720994                     /lib/x86_64-linux-gnu/libc-2.15.so
7f823d086000-7f823d08b000 rw-p 00000000 00:00 0 
7f823d08b000-7f823d0a0000 r-xp 00000000 08:03 30391                      /lib/x86_64-linux-gnu/libgcc_s.so.1
7f823d0a0000-7f823d29f000 ---p 00015000 08:03 30391                      /lib/x86_64-linux-gnu/libgcc_s.so.1
7f823d29f000-7f823d2a0000 r--p 00014000 08:03 30391                      /lib/x86_64-linux-gnu/libgcc_s.so.1
7f823d2a0000-7f823d2a1000 rw-p 00015000 08:03 30391                      /lib/x86_64-linux-gnu/libgcc_s.so.1
7f823d2a1000-7f823d383000 r-xp 00000000 08:03 390571                     /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.16
7f823d383000-7f823d582000 ---p 000e2000 08:03 390571                     /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.16
7f823d582000-7f823d58a000 r--p 000e1000 08:03 390571                     /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.16
7f823d58a000-7f823d58c000 rw-p 000e9000 08:03 390571                     /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.16
7f823d58c000-7f823d5a1000 rw-p 00000000 00:00 0 
7f823d5a1000-7f823d5c3000 r-xp 00000000 08:03 721006                     /lib/x86_64-linux-gnu/ld-2.15.so
7f823d796000-7f823d79b000 rw-p 00000000 00:00 0 
7f823d7bf000-7f823d7c3000 rw-p 00000000 00:00 0 
7f823d7c3000-7f823d7c4000 r--p 00022000 08:03 721006                     /lib/x86_64-linux-gnu/ld-2.15.so
7f823d7c4000-7f823d7c6000 rw-p 00023000 08:03 721006                     /lib/x86_64-linux-gnu/ld-2.15.so
7fff1f4c4000-7fff1f4e5000 rw-p 00000000 00:00 0                          [stack]
7fff1f564000-7fff1f565000 r-xp 00000000 00:00 0                          [vdso]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0                  [vsyscall]
Aborted (core dumped)

Is the syntax wrong? should what changes should I consider in the code?

Think about what happens if you were to do `{ A a; }` somewhere in your code. What would the destructor do? — fstd, May 02 '14 at 01:49
BTW, `delete nullptr` is perfectly safe. What you have here is not. — Ed S., May 02 '14 at 01:53
@fstd How can I make sure that a is allocated? the `if (a)` or `if (a!=NULL` didn't work — orezvani, May 02 '14 at 01:53
@emab [the memory a points to] is allocated after you allocated it:). Before that, a holds a garbage pointer, unless initialized (which you are not doing). Eventually you use delete[] on the garbage. — fstd, May 02 '14 at 01:54

score 1 · Answer 1 · answered May 02 '14 at 02:05

1

Your issue has absolutely nothing to do with vector. I can duplicate your problem without using vector.

Before going any further, try and make your class work for the following program:

int main()
{
   A a1;
   a1.initialize(1);
   A a2 = a1;
   A a3;
   a3 = a1;
}

This program must complete without error, without memory leaks, without crashing, without double deletes, etc. Once you get that small program working with your class, then and only then can you consider it to be placed in a container such as vector.

The program above does construction, copy/assignment, and destruction, all operations that vector will/may do to the objects placed inside of it.

A variation of the program above is to remove the call to a1.initialize and see what happens (this will test what happens if A is default constructed only).

answered May 02 '14 at 02:05

PaulMcKenzie

34,698
4
24
45

Your explanation is correct and understandable; But I need to push the object into `vector` and then initialize these values; Otherwise, vector will do this twice (once for creating this object and another time for copying it into the vector), using a great amount of time and memory; Do you think I should use an array instead of vector? – orezvani May 02 '14 at 02:10
Why not have your default constructor take an optional argument? `A(int x=0) : a(0) { if ( x > 0 ) initialize(x); }` `std::vector A(10, 4);` That would initialize all the entries with a value of 4. Also, you won't get any further benefit with an array. Anyway, how are you going to remove an item from an array? Arrays are fixed in size. – PaulMcKenzie May 02 '14 at 02:13
`using a great amount of time and memory` I have heard this so many times, and in almost every case, these alarms were a non-issue or were just plain false. – PaulMcKenzie May 02 '14 at 02:16
The reason that I didn't use the constructor was what I explained. Suppose the value of `i` is very large (say $10^8$) and every time it takes a few seconds to do the initialization; If I put it in the constructor, vector will double the running time (because it constructs two copies of this object). – orezvani May 02 '14 at 02:17
Check your implementation. Many do one `placement-new` call to allocate the entire memory pool, and then "fills" the pool with the objects. So only one set of objects is created. – PaulMcKenzie May 02 '14 at 02:29
In this example, `initialize` is just allocating memory which is not time consuming; in case that we have initialization of each element in `a` and other settings, this will be really time consuming. – orezvani May 02 '14 at 02:29
Instead of prematurely optimizing, why not test it? Make sure you compile with optimizations turned on, and see what happens. I have seen too many times persons guessing what a C++ implementation will do, and then waste their time "trying to beat the compiler" by writing code that is practically unmaintainable. – PaulMcKenzie May 02 '14 at 02:31
I already solved this problem; But I am testing to see ;) – orezvani May 02 '14 at 02:42
The problem that I was concerned about does not happen, because `c++` only creates some pointers to those allocated memory spaces (not copying them). I realized that it does not work for my application at all. – orezvani May 02 '14 at 07:07
I replaced `int *a` by a `list a`, and then tested practically. Using the `initializer` instead of constructor is two times faster; I can share the code with you. – orezvani May 03 '14 at 04:03

score 0 · Answer 2 · answered May 02 '14 at 02:01

0

This is not to imply that your way of creating that vector seems like a good idea, but the immediate problem I notice, is that you don't initialize the a member of your class A.
This /will/ blow up once the dtor is called, unless initialize() is called before.

answered May 02 '14 at 02:01

fstd

574
2
7

I think the `vector` is messing things up; It first creates one object, copies that object into the vector and then destroys it (which is not initialized). Is that correct? – orezvani May 02 '14 at 02:05
The onus is on you to make sure your object is copyable, by providing a copy-constructor and a copy-assignment operator. The vector just calls those functions (and if you don't define them, you get the compiler-generated versions, which are wrong for your class). If you want to disable those functions then make them `private` (or `= delete;` in C++11) and you will get a compilation error when you try to put your object in the vector. – M.M May 02 '14 at 02:36

score -1 · Answer 3 · answered May 02 '14 at 01:55

-1

The 10 pointer *a of The 10 objects in the vector are all point to the same address, so you got double free

answered May 02 '14 at 01:55

Icy

1

1

That does not seem correct. How did you arrive at that conclusion? – fstd May 02 '14 at 01:57
When I try creating a single object, I get `segmentation fault`; So, what's the explanation for that? – orezvani May 02 '14 at 01:59
@enab - That member pointer `a` is uninitialized. Then in your destructor you do this: `if (a) delete[] a;` Since `a` is uninitialized, it points to garbage. So you're calling `delete` on a garbage pointer. – PaulMcKenzie May 02 '14 at 02:01
I'm sorry i didn't see the main() . I thought the 10 objects were copying by the container just now. – Icy May 02 '14 at 02:12
hmm, i wonder why i get -1 rep for every comment to this answer (which isn't even mine) – fstd May 02 '14 at 02:23

Memory error while inserting an object into a vector

3 Answers3