How concerned should I be about the loss blocked in valgrind

Question

In my code I have a class lipid which holds three beads:

struct lipid{
  particle mainPart;
  bead * head;
  bead * body;
  bead * tail;
  int LID;
  vec direction;
  bead * operator[](int index){
    switch(index){
    case 0: return head;
    case 1: return body;
    case 2: return tail;
    default: return body;
    }
  }
};


struct bead{
  particle mainPart;
  int charge;
  int type;
  double rho;
  double nextRho;
  int LID;
  double U;
  double nextU;
  bool touch;
};


struct particle{
  vec pos;
  vec oldPos;
  vec vel;
  vec oldVel;
  vec F;
  vec oldF;
 };

class vec{
  velarry<double> coor;
  double x;
  double y;
  double z;
}

When I try to create a lipid, I create three beads using new

lipid * l = new lipid;
l->head = new bead;
l->body = new bead;
l->tail = new bead;

When I valgrind my code, I get an error which claims that there is loads of block missing... How concerned should I be about that? I should state that I'm pushing the beads and the lipids into (several) vectors.

Edit Ok, adding delete head .. fixed that but I still have an aching problem, I have a line:

this->beadBoxes[t[0]][t[1]][t[2]].push_back(b);

Where t is a vector<int> of size 3 and beadsBoxes is:

<vector<vector<vector<vector<bead*> > > > beadBoxes;

This guy gives me 5 times a memory leak error:

==22458== 48 bytes in 2 blocks are definitely lost in loss record 11 of 106
==22458==    at 0x4A0666E: operator new(unsigned long) (vg_replace_malloc.c:220)
==22458==    by 0x419A3C: __gnu_cxx::new_allocator<bead*>::allocate(unsigned long, void const*) (new_allocator.h:88)
==22458==    by 0x419A64: std::_Vector_base<bead*, std::allocator<bead*> >::_M_allocate(unsigned long) (stl_vector.h:127)
==22458==    by 0x423E1F: std::vector<bead*, std::allocator<bead*> >::_M_insert_aux(__gnu_cxx::__normal_iterator<bead**, std:\
:vector<bead*, std::allocator<bead*> > >, bead* const&) (vector.tcc:275)
==22458==    by 0x424073: std::vector<bead*, std::allocator<bead*> >::push_back(bead* const&) (stl_vector.h:610)
==22458==    by 0x409664: membrane::updateBox(bead*) (membrane.cpp:874)
==22458==    by 0x40ACA5: membrane::decide(lipid*) (membrane.cpp:793)
==22458==    by 0x40DF01: membrane::rotate() (membrane.cpp:529)
==22458==    by 0x40DFAF: membrane::MCstep() (membrane.cpp:480)
==22458==    by 0x401B54: main (main.cpp:15)

Which I suspect might be related to a segmentation fault which happens is that line. Why does the new (unsigned long) occur and why might it throw a segmentation fault?

Answer 1

Did you free the objects allocated with new?

For a consistent behavior you should consider using a Constructor and a Destructor:

struct lipid{
    // constructor
    lipid() {
        this->head = new bead;
        this->body = new bead;
        this->tail = new bead;
    }
    // destructor
    ~lipid() {
        delete this->head;
        delete this->body;
        delete this->tail;
    }
    ... // rest of the class

Answer 2

You should not be using Raw pointers in the first place.
Use Smart pointers & RAII, which is the best solution to the problem you are encountering. Using Smart pointers would ensure that the objects themselves will take care of deallocation once no pointer is referencing them anymore.

Have a look at shared_ptr.

As far as the source code you posted, You never deallocate the dynamically allocated objects, and hence the memory leak.

Answer 3

Of course you have a leak; you aren't freeing your memory.

Define a destructor in the lipid class that destroys the beads, and when you are storing the dynamically allocated lipids in a vector, use RAII with a shared_ptr or similar.

Better, use shared_ptrs for everything. Or even just use the stack; from the design you have given there's not really any need for pointers anyway (particularly naked pointers)

Answer 4

From your code, I suppose that what Valgrind is trying to tell you is that you have memory leaks.

After allocating some object on the heap:

lipid * l = new lipid;
l->head = new bead;
l->body = new bead;
l->tail = new bead;

you should also delete them appropriately to recover memory. Indeed, you lipid class is missing a destructor where you do clean up... you might try with something like this:

struct lipid{
  particle mainPart;
  bead * head;
  bead * body;
  bead * tail;
  ~lipid() {
      delete head;
      delete body;
      delete tail;
  }
  ...
};

and use it like this:

 lipid * l = new lipid;
l->head = new bead;
l->body = new bead;
l->tail = new bead;

...

//-- when you are done with lipid:
delete l;

Answer 5

Like the other posters excellently explained, these lost blocks are memory that was allocated, but never freed, and thus will never be usable again in the same run of your program.

When you try to run your program for a longer time and need many new lipid structures, this is a definitive problem. From a software engineering POV, you must deallocate your memory. However, you seem to program in a scientific context, and thus I'd like to add that your results are not influenced by the missing deallocation and, from the scientists' POV, you might be able to afford being sloppy here.