What can cause a segmentation fault using delete command in C++?

Zach picture Zach · Mar 22, 2011 · Viewed 30k times · Source

I've written a program that allocates a new object of the class T like this:

T* obj = new T(tid);  

where tid is an int

Somewhere else in my code, I'm trying to release the object I've allocated, which is inside a vector, using:

delete(myVec[i]);  

and then:

myVec[i] = NULL;

Sometimes it passes without any errors, and in some cases it causes a crash—a segmentation fault.

I've checked before calling delete, and that object is there—I haven't deleted it elsewhere before.

What can cause this crash?
This is my code for inserting objects of the type T to the vector:

_myVec is global

int add() {     

int tid =  _myVec.size();  
T* newT = new T (tid);  
    if (newT == NULL){  
        return ERR_CODE;  
    }  
    _myVec.push_back(newT);  
//  _myVec.push_back(new T (tid));  

    return tid;  
} 

as it is - the program sometimes crash.
When I replace the push_back line with the commented line, and leave the rest as it is-it works.

but when I replace this code with:

int add() { 

int tid =  _myVec.size();  
    if (newT == NULL){  
        return ERR_CODE;  
    }  
    _myVec.push_back(new T (tid));  

    return tid;  
}  

it crashes in a different stage...

the newT in the second option is unused, and still - changes the whole process... what is going on here?

Answer

Klaim picture Klaim · Mar 22, 2011

Segfaulting mean trying to manipulate a memory location that shouldn't be accessible to the application.

That means that your problem can come from three cases :

  1. Trying to do something with a pointer that points to NULL;
  2. Trying to do something with an uninitialized pointer;
  3. Trying to do something with a pointer that pointed to a now deleted object;

1) is easy to check so I assume you already do it as you nullify the pointers in the vector. If you don't do checks, then do it before the delete call. That will point the case where you are trying to delete an object twice. 3) can't happen if you set NULL to the pointer in the vector.

2) might happen too. In you case, you're using a std::vector, right? Make sure that implicit manipulations of the vector (like reallocation of the internal buffer when not big enough anymore) doesn't corrupt your list.

So, first check that you delete NULL pointers (note that delete(NULL) will not throw! it's the standard and valid behaviour! ) - in your case you shouldn't get to the point to try to delete(NULL). Then if it never happen, check that you're not having your vector fill with pointers pointing to trash. For example, you should make sure you're familiar with the [Remove-Erase idiom][1].


Now that you added some code I think I can see the problem :

int tid =  _myVec.size(); 

You're using indice as ids.

Now, all depends on the way you delete your objects. (please show it for a more complete answer)

  1. You just set the pointer to NULL.
  2. You remove the pointer from the vector.

If you only do 1), then it should be safe (if you don't bother having a vector that grows and never get released and ids aren't re-used).
If you do 2. then this is all wrong : each time you remove an object from a vector, all the object still contains after the removed object position will be lowered by one. Making any stored id/index invalid.

Make sure you're coherent on this point, it is certainly a source of errors.