31

The message:

terminate called after throwing an instance of 'std::bad_alloc'
what():  std::bad_alloc

I looked at the gdb backtrace and this is the lowest level method in there that I implemented myself:

/*
 * get an array of vec3s, which will be used for rendering the image
 */
vec3 *MarchingCubes::getVertexNormalArray(){
    // Used the same array size technique as getVertexArray: we want indices to match     up
    vec3 *array = new vec3[this->meshPoints.getNumFaces() * 3]; //3 vertices per face

    int j=0;
    for (unsigned int i=0; i < (this->meshPoints.getNumFaces() * 3); i++) {
        realVec normal = this->meshPoints.getNormalForVertex(i);
 //     PCReal* iter = normal.begin();

        if (normal.size() >= 3) {
            array[j++] = vec3(normal[0], normal[1], normal[2]);
        }
        cout << i << " ";
    }

    return array;
}

The cout statement you see above indicates that it terminates after 7000+ iterations. The above function is called only once near the end of my application. I call a very similar function before calling the above, that doesn't cause problems.

Rooster
  • 1,267
  • 3
  • 15
  • 23
  • 6
    Do you ever free the arrays returned by this function with `delete[]`? – Matteo Italia Dec 04 '11 at 21:42
  • 1
    How large is `meshPoints.getNumFaces()`? And how large is `vec3`? – celtschk Dec 04 '11 at 21:43
  • @MatteoItalia Oh, I'm not. And I'm also returning an equally sized array from another method. So I'll give that a try. – Rooster Dec 04 '11 at 21:44
  • 7
    Ouch, then you have massive memory leaks. By the way, you should use [smart pointers](http://stackoverflow.com/questions/569775/smart-pointers-boost-explained) or standard containers; you should avoid completely manual memory management in C++, your code will not be exception safe and you will always risk memory leaks. – Matteo Italia Dec 04 '11 at 21:47
  • @celtschk 'meshpoints.getNumFaces()' is 14,368. vec3 contains 3 GLfloat's. – Rooster Dec 04 '11 at 21:51
  • by using a standard container you're also less likely to throw away the count of valid elements (you have that available as `j`, then just throw it away) – Cheers and hth. - Alf Dec 04 '11 at 21:57
  • OK, assuming a GLfloat is 4 bytes (like a normal C float, IEEE single), then you tried to allocate 7000*14368*3*3*4 bytes, that is 3.45 gigabytes, just for those arrays. – celtschk Dec 04 '11 at 21:58
  • @MatteoItalia I can't delete the array if it terminates before being returned, can I (terminates on 7000 out of (14000*3))? – Rooster Dec 04 '11 at 21:59
  • @celtschk No, he's allocating 14368 * 3 * 3 * 4, he `new`s before the loop. The call that throws is probably in `this->meshPoints.getNormalForVertex(i);` – jrok Dec 04 '11 at 22:03
  • @bbarre: "if it terminates" - what is the subject here? :S – Matteo Italia Dec 04 '11 at 22:04
  • @bbarre: anyway, you have to `delete` it when you are done using it. Here it would be the caller's responsibility to do it, but keep in mind that the *real* solution is to use either smart pointers either "real" containers. – Matteo Italia Dec 04 '11 at 22:07
  • 1
    You shouldn't delete the array before it is returned, you should delete it when it is no longer needed. That is, somewhere in the code calling the function. Think of it as a blackboard: You write data on the blackboard, but you never wipe the blackboard to make place for new data (`delete[]`). If you do that long enough, at some time there's no place left on the blackboard. But you shouldn't wipe stuff from the blackboard which you still need; only stuff which is no longer needed. – celtschk Dec 04 '11 at 22:07
  • @celtschk This array is returned and passed to a GL vertex shader program, so I'm a little wary of when I no longer need it. This is one of the last major things that happens in my application, so would I just delete at the very end? – Rooster Dec 04 '11 at 22:12
  • @jrok: Ah, I see, I misread. I thought it was after about 7000 calls. – celtschk Dec 04 '11 at 22:16
  • Deleting at the very end wouldn't help. Since I (well, actually jrok) now noticed that I had misread: How often do you call that function before it fails? – celtschk Dec 04 '11 at 22:19
  • 2
    Another idea, in a completely different direction: Did you check that `j` always stays below `meshPoint.getNumFaces()*3`? Maybe you get the `bad_alloc` due to a buffer overrun overwriting essential heap structures. – celtschk Dec 04 '11 at 22:22
  • @celtschk Just once. I also get an equally sized array right before calling the above, once. I've verified that function doesn't cause problems. I'll add it to my question. – Rooster Dec 04 '11 at 22:23
  • 1
    Ah, OK, then this is probably not your problem. Deleting at the end of the program is not strictly necessary because at the end of the program the OS reclaims all memory from the program anyway, and your array contains only floats, so you're missing no destructor calls. So what about `j`? – celtschk Dec 04 '11 at 22:27
  • Ok, I just noticed that `j` cannot be larger than `i`, that is not larger than about 7000. What does `getNormalForVertex` do? – celtschk Dec 04 '11 at 22:33
  • @celtschk I appreciate you sticking with me -- should we move to chat or something? – Rooster Dec 04 '11 at 22:37
  • let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/5574/discussion-between-bbarre-and-celtschk) – Rooster Dec 04 '11 at 22:38

2 Answers2

37

(moving/expanding from the comments)

Since you are allocating a new array every time without deallocating it, you have a massive memory leak, i.e. you continue to ask memory to the system without ever giving it back. Eventually the space on the heap finishes, and at the next allocation all you get is a std::bad_alloc exception.

The "C-style" solution would be to remember to deallocate such memory when you don't need it anymore (with delete[]), but, this is (1) error-prone (think e.g. if you have multiple return paths inside a function) and (2) potentially exception-unsafe (every instruction becomes a potential return path if you have exceptions!). Thus, this way should be avoided.

The idiomatic C++ solution is to use either smart pointers - small objects that encapsulate the pointer and deallocate the associated memory when they are destroyed - or standard containers, that do more or less the same thing but with copy semantics and some more bells and whistles (including storing the size of the array inside them).

Community
  • 1
  • 1
Matteo Italia
  • 123,740
  • 17
  • 206
  • 299
4

My problem turned out to be that this->meshPoints.getNormalForVertex(i) accesses an array (or vector, I can't remember) whose length is less than this->meshPoints.getNumFaces() * 3. So it was accessing out of bounds.

Rooster
  • 1,267
  • 3
  • 15
  • 23
  • 17
    This does not explain, the std::bad_alloc exception: _Type of the exceptions thrown by the standard definitions of operator new and operator new[] when they fail to allocate the requested storage space._ – uceumern May 11 '16 at 06:46