Thanks Greg, for your comments. Will you allow me to pick your brain?
std::map<int, MyClass *> MyMap;
boost::shared_ptr<MyClass> GetInt( const int x )
{
std::map<int, MyClass *>::iterator where = MyMap.find( x );
if ( where == MyMap.end() )
{
MyMap[ x ] = new MyClass( x );
}
return boost::shared_ptr<MyClass>( MyMap[ x ],
MapElementReleaser(MyMap) );
}
This GetInt() routine has a serious flaw. GetInt() initializes a
shared_ptr with a raw pointer each time the routine is called.
Therefore none of the shared_ptrs that GetInt() returns for a
particular MyClass entry in the map, knows that any of the other
shared_ptrs previously returned by GetInt() to that same object exist.
So the first shared_ptr that goes out of scope will free its MyClass
pointer - no matter how many other shared_ptrs may also be holding
that same MyClass pointer. In other words, the program could easily
wind up with shared_ptrs that point to deallocated objects, and have
shared pointers attempt to delete pointers to objects that have
already been freed (or free pointers to new objects found at old
addresses that should not be freed).
Ah yes, I see. My little test was too narrowly focused to expose that flaw.
A program should assign a raw pointer to a shared_ptr only once. After
that point, the program simply uses the shared_ptr - and never uses
the raw pointer again.
Okay, so I should be creating the shared_ptr only when I am initially adding
an element to the map (which occurs only once)?
A better solution in this case, it seems to me, would be to store a
MyClass weak_pointer instead of a MyClass raw pointer in the map.
Storing a weak_pointer enables GetInt() to return shared_ptrs that
will actually share their pointers. In this way, the program willl not
free the MyClass object until the last - instead of the first -
shared_ptr goes out of scope.
As I stated in another post, I am not at all familiar with weak_ptr's and
don't really have any idea what changes would be necessary to use them in my
contrived sample program. But I'll start by changing the map definition:
typedef boost::weak_ptr<MyClass> MyClassPtr_t;
std::map<int, MyClassPtr_t> MyMap;
....and then modifying GetInt() to create a shared_ptr only once per map
element:
boost::shared_ptr<MyClass> GetInt( const int x )
{
std::map<int, MyClassPtr>::iterator where = MyMap.find( x );
if ( where == MyMap.end() )
{
MyMap[ x ] = boost::shared_ptr<MyClass>( new MyClass( x ),
MapElementReleaser(MyMap) );
}
return MyMap[ x ].lock(); // is this correct?
}
The above compiles...but I have a couple of questions:
1) Is it okay to directly assign a shared_ptr to a weak_ptr as I have done
above, or do I need to be more explicit by creating a weak_ptr first and
assigning it to the map afterward?
2) Am I thinking correctly about the return value? It is still a shared_ptr
(which I still what I want, right?), and lock() returns a shared_ptr and
increments its reference count (as if I were copying it)?
Now what is the correct way to write the custom deleter? The custom deleter
below compiles and functions as expected when MyClassPtr is a raw ptr, but
not when it is a weak_ptr:
class MapElementReleaser
{
private:
std::map<int, MyClassPtr> &FMap;
public:
explicit MapElementReleaser( std::map<int, MyClassPtr> &AMap )
: FMap( AMap ) {}
void operator()(MyClassPtr pMyClass) const
{
for ( std::map<int, MyClassPtr>::iterator map_end( FMap.end() ),
map_iter( FMap.begin() );
map_iter != map_end;
++map_iter )
{
if ( map_iter->second == pMyClass )
{
FMap.erase( map_iter->first );
break;
}
}
}
};
The idea behind the operator() here is that it must remove the element in
the map that contains the weak_ptr identified by 'MyClassPtr pMyClass.' I
thought perhaps I could do something like:
if ( map_iter->second.lock().get() == pMyClass.lock().get() )
....but that won't compile (and the error messages don't make any sense to
me!). Furthermore, it seems to me that it would increment the reference
count on the very shared_ptr that is currently being deleted via the custom
deleter (which can only occur when its reference count has already reached
zero)! So, what is the best way to find and remove the appropriate element?
Thanks,
- Dennis