C++ std::vector of pointers deletion and segmentation faults

Question

Hello all,

I have a vector of pointers to a class. I need to call their destructors and free their memory. Since they are vector of pointers vector.clear() does not do the job.So I went on to do it manually like so :

void Population::clearPool(std::vector<Chromosome*> a,int size)
{
    Chromosome* c;
    for(int j = 0 ;j < size-1;j++)
    {
       c = a.back();
       a.pop_back();
       delete c;
       printf("  %d \n\r",j);
       c = NULL;

    }

}

The printf in there is since I have a talking destructor to see in which Chromosome the segmentation fault happens. When clearPool() is called and say we got a size of 100, it can give a segmentation fault in any Chromosome between 0 and 100. I have no idea why this might be happening nor do I have a way to actually find what's wrong since while debugging with breakpoints all I see is that it happens in there at random chromosomes.

Any help would be really appreciated. I am using codeblocks IDE and the gdb debugger. The stack trace when the segmentation fault happens has 4 memory addresses and a function: 'wsncpy()'.

Thanks in advance, -Lefteris

Answer 1

void Population::clearPool(std::vector<Chromosome*>& a)
{
    for(size_t i = 0; i < a.size(); i++) {
     delete a[i];
    }

    a.clear();
}

Answer 2

void Population::clearPool( std::vector <Chromosome*> & a )
{
   for ( int i = 0; i < a.size(); i++ ) {
      delete a[i];
   }
   a.clear();
}

Notice that the vector is passed by reference. In your code, a copy of the vector is used, which means that it is unchanged in the calling program. Because you delete the pointers in the copy, the pointers in the original are now all invalid - I suspect you are using those invalid pointers in some way not shown in the code you posted.

As a couple of template solutions have been posted that use C++ library algorithms, you might also want to consider a template solution that does not:

template <class C> void FreeClear( C & cntr ) {
    for ( typename C::iterator it = cntr.begin(); 
              it != cntr.end(); ++it ) {
     delete * it;
    }
    cntr.clear();
}

Using this you can free any container of dynamically allocated objects:

vector <Chromosome *> vc;
list <Chromosome *> lc;
// populate & use
FreeClear( lc );
FreeClear( vc );

Answer 3

Are you sure that each pointer in the vector points to a different object? (i.e. that two pointers don't both point to the same object, which you're trying to delete twice.

Are you sure that you don't delete some of the pointers before calling this method? (i.e. are you sure that each pointer in the list points to a valid object?)

Answer 4

I don't know why you are crashing, but I guess that one possibility is that the size of the vector is not the same as the size you are passing in. Also I notice you are iterating from 0 to size-2, do you not mean to go all the way to the end?

One way to delete all of the items in the array using idiomatic C++ is something like this:

template<class T>
class deleter
{
  public:
    void operator()(const T* it) const
    {
      delete it;
    }
};

std::for_each(a.begin(), a.end(), deleter<Chromosome>());

Answer 5

Slight modified version compared to (@1800 INFORMATION).

  struct DeleteFromVector
    {
        template <class T>
        void operator() ( T* ptr) const
        {
         delete ptr;
        }
    };


std::for_each(aVec.begin(), aVec.end(), DeleteFromVector());

Answer 6

I recommend to use smart pointer (ie:auto_ptr) instead of raw pointer and just use vector::clear method that will call the destructor for each element

Answer 7

It seems, that some pointers in your code do not reference correct Chromosome objects. This may happen, if you try to delete some objects twice as a result of code:

Population p;
vector<Chromosome*> chromosomes;
p.clearPool(chromosomes,chromosomes.size()); // You pass by value, so chromosomes is not changed
p.clearPool(chromosomes,chromosomes.size()); // Delete already deleted objects second time

You may find useful ptr_vector from Boost Pointer Container Library in order to avoid similar errors

Answer 8

Boost lambda already has a functor for deleting sequences of pointers, by the way:

std::for_each(a.begin(), a.end(), boost::lambda::delete_ptr());

Answer 9

The most likely reason is calling delete twice for the same address. This can happen if you added one object more than once to the vector. To detect this insert some statement that will output the address of the object you will then delete.

printf( "will delete %d\n", (int)c );
delete c;

Answer 10

I found the problem.

It was in the most well hidden (by none other than stupid old me) place it could be.

As some might have guessed this is a genetic algorithms program. It is for a tutorial I am making. I was choosing the crossover points for the chromosomes randomly from a roulette wheel function which I made. Well ... inside there, there was a -1 which should not be there. That destroyed literally everything, and eventually lead to a segmentation fault.

Thank you all for your help, I saw some really good practises in this post which I intend to follow