Vector of shared pointers

benjymous

I had a container class that had a member of shared pointers to a grid of cells

class CCell;
typedef boost::shared_ptr< CCell > CellPtr;
typedef std::vector< CellPtr> CellArray;

class Container
{
Container()
~Container()

..logic code snipped for simplicity..

private:
CellArray m_apxCells;
}

Now, as I was using shared_ptr's, I was working under the assumption that I didn't need to worry about clearing up my CellArray - boost would automatically delete everything when the container class was deleted... ... But instead I got massive memory leaks from all the cells ... Hint: here's the code from the Constructor

Container::Container(void)
{
// Initialise the cells
for( unsigned int n=0; n<nNumRegions; ++n )
{
m_apxCells.push_back( CellPtr( new CCell( n ) ) );
}

// Initialise the neighbours

for( unsigned int nY=0; nY<nMapSize; ++nY )
{
for( unsigned int nX=0; nX<nMapSize; ++nX )
{
CellPtr cell = GetCell( nX, nY );
RI_ASSERT( cell != NULL_REGION );

  for( int nDY = -1; nDY <= 1; ++nDY )
  {
    for( int nDX = -1; nDX <= 1; ++nDX )
    {
      // Don't add cell as a neighbour of itself
      if( nDY != 0 || nDX != 0 )
      {
        CellPtr neighbour = GetCell( nX + nDX, nY + nDY );
        if( neighbour != NULL\_REGION )
        {
          cell->AddNeighbour( neighbour );
        }
      }
    }
  }
}

}
}

-- Help me! I'm turning into a grapefruit! Buzzwords!

Chris Meech

In the destructor, you need to delete the CCell objects that were instantiated. The container will only clean up the memory it allocated for storing pointers to the CCell objects, not the objects themselves. Is that the problem?

Chris Meech I am Canadian. [heard in a local bar] I agree with you that my argument is useless. [Red Stateler] Hey, I am part of a special bread, we are called smart people [Captain See Sharp] The zen of the soapbox is hard to attain...[Jörgen Sigvardsson] I wish I could remember what it was like to only have a short term memory.[David Kentley]

benjymous

Nope - usually the vector will happily destroy and clean up - it's all to do with the way the cells store pointers to their neighbours

-- Help me! I'm turning into a grapefruit! Buzzwords!

Nemanja Trifunovic

Circular references, right? neighbour should be a weak pointer, not shared_ptr

Programming Blog utf8-cpp

Chris Meech

So either the ctor for CCell class or the AddNeighbour method is also doing a 'new' that isn't deleted when the dtor for CCell is called? But that wouldn't explain why this is a bug with the vector would it?

Chris Meech I am Canadian. [heard in a local bar] I agree with you that my argument is useless. [Red Stateler] Hey, I am part of a special bread, we are called smart people [Captain See Sharp] The zen of the soapbox is hard to attain...[Jörgen Sigvardsson] I wish I could remember what it was like to only have a short term memory.[David Kentley]

benjymous

Yup, that's right All the cell's neighbour pointers keep references alive boost::shared_ptr waits for the reference to reach zero before it deletes the object, but none of the references will ever automatically reach zero, because of all of the neighbour pointers. The solution was to iterate the cells in the container's destructor and tell each one to clear its neighbour vector. Once that was done the container's cell vector destructs itself as expected (I'll try weak pointers instead, to see if it makes it simpler!)

-- Help me! I'm turning into a grapefruit! Buzzwords!

Pierre Leclercq

This is a very good example of the care that should be taken with reference counting. Using shared_ptr and weak_ptr allows for a clear separation between who owns the pointer and who uses it. On the other hand, using a shared_ptr for just owning a pointer could be simplified to just a pointer owner in many cases. Does somebody know wether boost provides such a class? (It is simple to write, but IMO this is a useful paradigm).

jhwurmbach

Pierre Leclercq wrote:

On the other hand, using a shared_ptr for just owning a pointer could be simplified to just a pointer owner in many cases. Does somebody know wether boost provides such a class?

boost has boost::scoped_ptr[^]: "Because it is noncopyable, it is safer than shared_ptr or std::auto_ptr for pointers which should not be copied."

"We trained hard, but it seemed that every time we were beginning to form up into teams we would be reorganised. I was to learn later in life that we tend to meet any new situation by reorganising: and a wonderful method it can be for creating the illusion of progress, while producing confusion, inefficiency and demoralisation." -- Caius Petronius, Roman Consul, 66 A.D.

Pierre Leclercq

Thanks. Have you tried to install Boost with VC8? I'd be glad to hear about it.

jhwurmbach wrote:

We trained hard, but it seemed that every time we were beginning to form up into teams we would be reorganised.

That Caius Petronius rocks! Some things will never change :)

Lost User

Pierre Leclercq wrote:

Have you tried to install Boost with VC8? I'd be glad to hear about it.

Yes, and haven't had any serious issues other than a few warnings that you can opt to disable via a #pragma. (I tend to use a special stdstl.h for my apps that includes all the Boost/STL goodness I am likely to need). BTW, FFIW I am using BOOST_FOREACH (which will be included in the 1.34 release) and it really is very useful indeed. :)

Zac Howland

Chris Meech wrote:

So either the ctor for CCell class or the AddNeighbour method is also doing a 'new' that isn't deleted when the dtor for CCell is called? But that wouldn't explain why this is a bug with the vector would it?

The bug isn't with vector. He is using shared_ptr's (a smart pointer class that cleans up after itself). delete does not need to be called (as it will be called in the shared_ptr's destructor). The problem is most likely the need to use a weak_ptr when connecting the neighbors so that you avoid the problem where 2 pointers are pointing to each other but have nothing pointing to either of them.

If you decide to become a software engineer, you are signing up to have a 1/2" piece of silicon tell you exactly how stupid you really are for 8 hours a day, 5 days a week Zac

jhwurmbach

Pierre Leclercq wrote:

Have you tried to install Boost with VC8? I'd be glad to hear about it.

Not myself. I am still using VC7.1. But in our company boost is in heavy use, and a few projects are using VC8. No problems that I have heard of.

Pierre Leclercq wrote:

That Caius Petronius rocks! Some things will never change

Actually, it seems not to be a genuine antique proverb. It probably was made up in WWII to be a Caius Petronius look-alike in style and spirit. But it is so very true...:^)

"We trained hard, but it seemed that every time we were beginning to form up into teams we would be reorganised. I was to learn later in life that we tend to meet any new situation by reorganising: and a wonderful method it can be for creating the illusion of progress, while producing confusion, inefficiency and demoralisation." -- Caius Petronius, Roman Consul, 66 A.D.

Robert Bielik

I'm using boost v1.33.1, works fine for me...

dre767

You should definately use boost::weak_ptr for your neighbours. They are suited for what you are trying to do, because can only refer to another boost::shared_ptr or boost::weak_ptr good luck