4

The problem was a stupid error from another class accessing the vector and deleting iterators. Nothing to do with the code below. Sorry to waste your time.

I must be missing something elementary. I've got a function which creates an object, manipulates it's data and then pushes it into a vector. The moment the function exits, the program crashes with a SIGSEV, and I'm left staring at (Kdevelop gcc 4.5 gdb) :

   /**
   *  The dtor only erases the elements, and note that if the
   *  elements themselves are pointers, the pointed-to memory is
   *  not touched in any way.  Managing the pointer is the user's
   *  responsibility.
   */
  ~vector()
  { std::_Destroy(this->_M_impl._M_start, this->_M_impl._M_finish,
          _M_get_Tp_allocator()); }

I am not storing pointers, I'm trying to store instantiated objects.

void Init::initIndividual(int ID, int gen)
{
  Individual temp_person = Individual(ID,gen);
  int inst_size = getRandom<int>(1,max_inst_size);
  for (int k=0;k<inst_size;k++)
  {
    retry:
    // (1) randomly choose a body part
    int body_num = getRandom<int>(1,20);
    body_part temp_part = get_body_part(body_num);
    // NOTE: We need to make sure that the body part is unique!
    std::vector<Instruction> already_existing = temp_person.get_instructions();
    if (already_existing.size() > 0)
    {
      for (int a=0; a< already_existing.size();a++)
      {
       std::string name = already_existing[a].get_body_part();
       if ( name.compare(temp_part.name) == 0 )
       { //if body part already exists in the list, retry!
         goto retry;
       }
      }
    }    
    // (2) Create a new Instruction for this body part
    Instruction temp_inst = Instruction(temp_part.name,temp_part.max_angle,temp_part.min_angle);
    // (3) Randomly pick a number of body parameters to use
    int paramsize = getRandom<int>(1,max_params_size);
    // (4) Randomly choose time and degree trajectory parameters for this body part and append!
    for (int x=0;x < paramsize; x++)
    {
     float time = 0.0f;
     int choice = 0;
     // (4.a) If begin of body parameters
     if (x==0)
     {
   //if always start at time = 0
   if (static_time_init)
   {
     time = 0.0f;
   }
   //if randomly choose the init time
   else if (!static_time_init)
   {
     time = getRandom<float>(0.0f,(float)(time_constrain-1));
   }
     }
     // (4.b) if not @ start of params
     else if(x!=0)
     {
       redo:  
       float previous_time = temp_inst.parameters.back().time; //get previous time
       double incrementor = getRandom<double>(0.1,1.0); //increment time by min 0.1 max 1.0
       time = previous_time + (float)incrementor;
       if (time > time_constrain) //if current time is more than time constrain, redo
       { 
        goto redo;
       }
     }
     // (5) Randomly pick a degree to move to (within body part constrains)
     float degree = getRandom<float>(temp_inst.get_min_angle(),temp_inst.get_max_angle());
     Parameter foo = Parameter(time,degree);
     temp_inst.add_parameter(Parameter(time,degree));
   }
  temp_person.add_Instruction(temp_inst);
  }
  temp_person.endtime = time_constrain;
 }

That is the entire function.

 std::vector<Individual> population;

Doesn't the push_back function copy the object when pushing it back ? Is the destructor invoked because push_back is trying to destroy temp_person ? I have not defined a copy operator in class Individual. I have run into this problem before and never figured it out. Does this happen because at the end of the function temp_person is out of scope ? Thank you !

Edit: Class Individual

class Individual 
{
   friend class Population;
   friend class Crossover;
   friend class Init;
 private:
   std::string xml_file;
   char *arg4;
 protected:
   bool saved, mutated, dead;
   unsigned int UID, generation;
   int executions;
   std::vector<Instruction> instructions;
   int father_UID, mother_UID;
   double eta,endtime;
 public:
   int uniform;
   float fitness;
   pthread_mutex_t thread_mutex;
   //Some other functions irrelevant

Please note, that the vector of instructions has another vector of structs.

class Instruction 
{
  friend class Crossover;
 private:  
  unsigned int param_size;
  float max_angle, min_angle;
  bool micro_mutated;
 public:
  std::string body_part;
  std::vector<Parameter> parameters;
  //other stuff

class Parameter
{
  public:
   float time;
   float degree;
   Parameter(float t,float d);
};

Nothing crazy here.

Could this be a problem of a shallow copy obtained by the population.push_back ?

Bo Persson
  • 90,663
  • 31
  • 146
  • 203
Ælex
  • 14,432
  • 20
  • 88
  • 129
  • 2
    Does the class `Individual` follow the [rule of three](http://stackoverflow.com/questions/4172722/)? Does it have any raw pointers pointing to heap memory or other handles to resources that are released in the destructor? – fredoverflow Jun 24 '11 at 17:47
  • Can you post the definition of `Individual`? Specifically, does `Individual` contain a vector in some form? – dolphy Jun 24 '11 at 17:48
  • Yes Individual contains vectors of other classes. Could this be it ? I'll post it below the question. – Ælex Jun 24 '11 at 17:50
  • We'll probably need the constructor/destructor as well, at a minimum. My assumption is that you're not having a problem with `population`, you're having a problem with the destruction of `temp_person` as it leaves scope. Something in your generation of an `Individual` is unsafe with respect to cleanup. – dolphy Jun 24 '11 at 17:55
  • I have not declared a destructor, only two constructors for individual – Ælex Jun 24 '11 at 17:56
  • You have a mutex in your `Individual` class. Does the class constructor create one, and the class destructor delete one? How do you deal with copying mutexes? – Nicol Bolas Jun 24 '11 at 17:58
  • Post them, and the same may very well apply to `Instruction` and `Parameter` (and possibly even deeper), depending on what we find in your `Individual` constructor. The bottom line is: trace exactly what's happening in the creation of an `Individual`, and then start working *backwards* to see exactly what's happening in the destruction of everything. – dolphy Jun 24 '11 at 18:02
  • 2
    The crickets you hear chirping are a result of everyone reading and re-reading to make sure they are seeing the word `goto`. – dolphy Jun 24 '11 at 18:16
  • The mutexes are initialized from the Constructor (and sadly never deleted cuz I haven't implemented a Destructor yet). BTW, what is so evil about goto's ? – Ælex Jun 24 '11 at 18:58
  • 1
    I think `goto` can have its uses, but it has to be extremely justified...(more than just convenience or cursory performance). All of those reasons would *probably* involve the words "political", or "legacy". [Here's a discussion!](http://stackoverflow.com/questions/46586/goto-still-considered-harmful). Regardless, I'm glad you found the issue. – dolphy Jun 24 '11 at 19:25

3 Answers3

6
pthread_mutex_t thread_mutex;

Copying a pthread_mutex_t is not meaningful. I'd wager that's part of the problem. I have suspicions about your char* too; who owns it?

My justification as to why I believe pthread_mutex_t can't be copied: the only documented way to obtain an initialized mutex is from using pthread_mutex_init. Additionally, all the pthread_mutex_* functions manipulate mutexes using pass-by-pointer (unlike e.g. pthread_thread_t).

Luc Danton
  • 34,649
  • 6
  • 70
  • 114
  • I didn't know its illegal to copy them, but threads are not used at this point. – Ælex Jun 24 '11 at 18:02
  • The pthread_mutex is initialized in the constructor. I can move this elsewhere since I use the threads later on for execution of many individuals at once. Nevertheless I do need a pthread_mutex belonging to the Individual. char * is uninitialized, it's used as a temp parameter, I could do without it. – Ælex Jun 24 '11 at 18:34
2
Doesn't the push_back function copy the object when pushing it back ?

Yes, it does.

Is the destructor invoked because push_back is trying to destroy temp_person?

No, it is not. It is called because the temp object is destroyed at the end of the function.

I have not defined a copy operator in class Individual.

Let us see the declaration of the class.

Does this happen because at the end of the function temp_person is out of scope ?

Yes, the temp object is destroyed, but if the class is OK, this should not be a problem. Again - we need to see the code.

I bet it's something with the constructor/destructor (:

By the way, is population global? Outch!

Kiril Kirov
  • 37,467
  • 22
  • 115
  • 187
  • No, the entire code is part of another class who's private variables contain population. I've posted the rest of the classes, I guess I should also post the entire function that causes the problem ? – Ælex Jun 24 '11 at 17:55
  • @Alex - yes, please paste the whole problem function and let us know if it's public or it can be called from a public. I see mutex-es here, so it's multithreaded application. Are you sure everything's thread-safe? Also, what is `char *arg4` for? How you deal with it? Do you use `new[]`/`delete[]` to allocate memory for it? If so, then you **must** to have some copy constructor to handle it properly, when copying the object (pushing it back into the `vector`) – Kiril Kirov Jun 24 '11 at 17:59
  • There are no threads used at this point, it's all initialization used random values. I have not used new or delete anywhere in this project. char *arg4 is temporarily used to pass some values to a serializer. I'll post the entire function as it is. – Ælex Jun 24 '11 at 18:02
  • Could it be a shallow copy issue ? – Ælex Jun 24 '11 at 18:10
  • At the beginning, I thought about that, but as you say, that `char* arg4` is used properly :? Did you try to remove the mutex, as @Luc Danton said? I don't think it's unsafe, but I don't know for sure. – Kiril Kirov Jun 24 '11 at 18:16
  • Now I'm getting an std::bad_alloc ! lol No, it's not the mutex, I tried with and without it. Probably something in the code that accesses the vectors – Ælex Jun 24 '11 at 18:31
  • `std::bad_alloc` - lol, that's really shocking :D Hmmm, what is this class `Parameter` ? Let's see its declaration. Also, the constructor of `Individual`, `Instruction` and `Parameter`. Lol, that's a lot of code, I guess :D Now I have another theory - something's wrong with `Parameter` (hint - the `std::vector parameters` in `Instructions`) – Kiril Kirov Jun 24 '11 at 18:39
  • I think the problem is in Population. It tries to copy the vector from the init class, and somehow it doesn't. The Individuals are now initialized without crashing, all I did was comment out the pthread_mutex initialization in the constructor. – Ælex Jun 24 '11 at 18:41
  • I fixed it, please don't hate me, it was a stupid mistake in Population. Nothing to do with vectors. Really sorry for wasting your time. BTW, what would be a more efficient way do this init function without creating an object and then copying it ? – Ælex Jun 24 '11 at 18:46
  • 1
    @Alex - haha, take it easy, nobody's gonna hate you :D What was the error, I'm curious? About the init function - well, I can't say, without knowing the whole logic and without seeing the whole picture, which I can't, as it's too much code. But I'd advise to redesign your code, or at least this function. It looks too complicated and once one code got complicated - then there's something wrong with the logic :p And, by the way, these `goto`-s scare me, a little :D – Kiril Kirov Jun 24 '11 at 18:52
  • There was another thread running which was removing "dead" individuals and resizing the vector and invalidating some iterators. As soon I disabled it it worked fine. The goto's are a necessary evil in this case, I could do it with loops but it would mean even more code. Thank you very much for the help and for answering my questions ! – Ælex Jun 24 '11 at 18:56
  • You're welcome (: So, it appeared to be a multithread issue, my first suspection :P Good luck with the app (= – Kiril Kirov Jun 24 '11 at 19:43
0

The problem was a stupid error from another class accessing the vector and deleting iterators. Nothing to do with the code above. Sorry to waste your time.

Ælex
  • 14,432
  • 20
  • 88
  • 129