Memory leak, vector pushback c++

Question

I have a class Bar, its constructor initializes a std::vector of type Foo (another class).

Bar.cpp

Bar::Bar(int n) {
 for(int i = 0; i < n; i++) {
    Foo foo;
    foo.somefunction(i);
    vec.push_back(foo) //this should insert foo into the vector
  }
}

Bar.h

class Foo;
class Bar {
 std::vector<Foo> vec;
};

When I debug, the first iteration of the construction works fine. foo is created, foo.somefunction() is run fine, and foo is pushed into vec.

The second interation seems to work fine as well, but the program crashes when it goes back to start the third iteration.

I get _BLOCK_TYPE_IS_VALID(pHead->nBlockUse) error and HEAP CORRUPTION DETECTED.

Foo is a class that contains an dynamically created array, nothing special. Something like this:

Foo::Foo() {
  solution = new float[size];
  // some function that initializes the elements of solution
}

and a regular destructor ~Foo() {delete [] solution;}. I don't think the problem comes from Foo.

You've used a vector of `Foo`s. Why wouldn't you used a vector of `float`s, too? I bet you problem will magically disappear. — jrok, Mar 13 '13 at 17:27
I tried declaring a vector of float in this way: std::vector vec (size); but it doesn't work. Any way to declare a vector of a size determined at run-time? — jazzybazz, Mar 13 '13 at 17:50
@jazzybazz: Vectors are dynamic by nature. You don't have to give them a size. But if you want to give one an initial size, use a constructor initializer to do it. E.g., `Foo::Foo() : solution(size) { ... }`. — Fred Larson, Mar 13 '13 at 18:50

score 4 · Answer 1 · edited May 23 '17 at 11:51

4

Most likely you did not implement the copy constructor and operator = properly and you are double deleting solution. As mentioned you should also read up on The Rule of Three.

The C++ standard containers store copies and so when you do a push_back you are making a copy.

edited May 23 '17 at 11:51

Community

1
1

answered Mar 13 '13 at 17:26

Shafik Yaghmour

154,301
39
440
740

Thank you, I think I will get rid of the dynamic array and use a vector of float instead. – jazzybazz Mar 13 '13 at 17:49
@jazzybazz That would probably be simplest solution – Shafik Yaghmour Mar 13 '13 at 20:22

score 0 · Answer 2 · answered Mar 13 '13 at 18:01

It looks like you didn't implement copy constructor in class Foo. In Bar constructor each iteration new instance of Foo is created and destroyed when the iteration is over. So the memory allocated in Foo is destroyed, but default copy constructor that copied instance of Foo to vector didn't copy the memory you allocate with "new", just copied the pointer. So after each iteration each vector element is corrupted. Let assume that vector had some allocated memory for your objects at the beginning. Then when there was no place in the buffer and he couldn't grow that buffer any more, vector allocates new memory and another copy operation happens. When copy operation is finished old buffer need to be freed and vector destroys all objects in old buffer and destructor in each object will call delete[] on corrupted pointer.

Memory leak, vector pushback c++

2 Answers2