Pool Class throwing error

Question

I am trying to implement a Pool Class that maintains a pool of linked list nodes

Although allocation and deallocation are working correctly, destructor is throwing an exception.

class Pool {
public:

  Pool ();

  ~Pool ();

  tEmployee *GetFromPool (void);

  void GiveToPool (tEmployee * pNode);

  void  PrintPoolSize ();

private:
  int vTop;                        
  tEmployee *vPool;                 
  tEmployee *vDeleted;
};

Here are the implementation of functions

Pool::Pool () 
  :vTop (0), vDeleted (NULL)
{
  vPool = new tEmployee[MAX_POOL];
}

tEmployee* Pool::GetFromPool (void) 
{
  if (vDeleted) {
    tEmployee * temp = vDeleted;
    vDeleted = vDeleted->next;

    return temp;
  }

  if (vTop == MAX_POOL) {

    vPool = new tEmployee[MAX_POOL];
    vTop = 0;
  }

  return vPool + vTop++;
}

void Pool::GiveToPool (tEmployee * pNode)
{
  pNode->next = vDeleted;

  vDeleted = pNode;
}

Pool::~Pool ()
{   
  tEmployee *curr = vDeleted;
  tEmployee *next = 0;

  while (curr) {

    next = curr->next;
    delete curr;    //This line is throwing exception on the second iteration of the loop
    curr = next;
  }

  delete [] vPool;
}

Is it due to heap corruption?

Does the constructor of `tEmployee` initialize the `next` pointer of that type to `NULL`? — sth, Dec 01 '11 at 19:20
@SumitJain: It's good to know how they work, and what their performance characteristics are, but be aware you should never, ever implement your own linked list in C++. Use `std::list` for any production code. And only when you actually need it, which probably won't be very often. — Cat Plus Plus, Dec 01 '11 at 19:22
I'm pretty sure I want to see the tEmployee class as well for a hint about its next pointers. — Michael Dorgan, Dec 01 '11 at 19:24
tEmployee is not a class, just a doubly linked list structure with no functions — Sumit Jain, Dec 01 '11 at 19:34

Mike Seymour · Accepted Answer · 2011-12-01T19:36:28.617

2

You allocate an array of employees:

vPool = new tEmployee[MAX_POOL];

and then incorrectly attempt to delete them individually:

delete curr; // Don't do this

before correctly deleting the array:

delete [] vPool;

As a general rule, each new must be matched with one delete; you didn't new the employees individually, so don't delete them individually.

You'll also need to maintain a list of pointers to all the arrays you allocate, so you can delete them all in the destructor; currently, you leak all of them except the last one you allocated. I would suggest something like:

std::vector<tEmployee *> vPool; // store all allocated blocks

tEmployee* GetFromPool() {
   if (vDeleted) {
       tEmployee * temp = vDeleted;
       vDeleted = vDeleted->next;
       return temp;
  }

  if (vTop == MAX_POOL) {    
    vPool.push_back(new tEmployee[MAX_POOL]); // add new block to collection
    vTop = 0;
  }

  return vPool.back() + vTop++;
}

~Pool() {
    for (size_t i = 0; i < vPool.size(); ++i)
        delete vPool[i];
}

edited Dec 01 '11 at 19:36

answered Dec 01 '11 at 19:22

Mike Seymour

249,747
28
448
644

okay..but but i am storing them in a linked list vDeleted....how can I delete all at once as vPool is allocating new memory when the pool becomes empty...and the recycled ones are going to vDeleted – Sumit Jain Dec 01 '11 at 19:27
1

@SumitJain: see the example code I've added. The important point is that you *don't* individually delete the items in `vDeleted` - they are in one of the arrays you allocated, and get deleted with the array. – Mike Seymour Dec 01 '11 at 19:37
can you explain how i am leaking them as when GiveToPool is called I store them in linked list and then delete this linked list in destructor – Sumit Jain Dec 01 '11 at 19:40
@SumitJain: Also remember the [rule of three](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three). If your class is managing resources, and so has a non-trivial destructor, then you should either provide or remove the copy constructor and copy assignment operator. – Mike Seymour Dec 01 '11 at 19:47

Pool Class throwing error

1 Answers1