boost::shared_ptr, std::map and valgrind - Do I have a memory leak?

Alright. I am using boost::shared_ptr to store a couple of objects in a map. Integer values maps to shared_ptrs to objects that I am using.

void HandlerMsgHandler::addHandler(uint8_t key, boost::shared_ptr<NetworkHandler> handler) 
{
    handlers[key] = handler; 
}

This code blows up in valgrind, look below for the error message. If you look at the beginning of the method chain, you can see that in the UdpServer class I forward the addHandler request from my testclass into an inner class called HandlerMsgHandler.

This is how that method looks like:

void UdpServer::addHandler(uint8_t key, boost::shared_ptr<NetworkHandler> handler)
{
    dynamic_cast<HandlerMsgHandler*>(sysMsgHandlers[network::handler])->addHandler(key, handler);
}

And the callsite:

server.addHandler(1, boost::shared_ptr<net::NetworkHandler>(new _NetworkHandler(networkHandlerFailed)));
server.addHandler(2, boost::shared_ptr<net::NetworkHandler>(new SimpleIntMessageHandler(simpleIntFailed)));

I need to cast because I have client message handlers, and network/system message handlers which handle certain other cases such as errors. The HandlerMsgHandler is invoked when a valid message has been sent to a client NetworkHandler.

I'm not sure how much you need to know about the design, perhaps the mistake is obvious to you?

UPDATE:

sysMsgHandlers is declared as std::map<SysMessage, NetworkHandler*> sysMsgHandlers;, while the sysMsgHandlers are initiated internally as such:

sysMsgHandlers[handler] = new HandlerMsgHandler();
sysMsgHandlers[error] = new ErrorMsgHandler();

I don't use any fancy pointer types here since it's all internal. I iterate over the sysMsgHandlers map and delete the pointers (I have verified that this happens).

UPDATE 2:

More info. I am deleting the object containing the map.

I have this code in my UdpServer destructor:

for(auto iter = sysMsgHandlers.begin(); iter != sysMsgHandlers.end(); iter++)
{
LOG("MsgHandlers deleted");
delete iter->second;
}

This gives the following output to std::cout: (among a few other debug messages)

 : Listening started
 : Connecting to socket : 1
 : Sending message
 : Received in buffer
 : Received : 24 bytes.
 : Sys msg = 1
 : Handling request to: 1
 : Stopping manager
 : MsgHandlers deleted
 : MsgHandlers deleted

And this is still the valgrind output:

==26633== 192 (56 direct, 136 indirect) bytes in 1 blocks are definitely lost in loss record 21 of 35
==26633==    at 0x4C28973: operator new(unsigned long) (vg_replace_malloc.c:261)
==26633==    by 0x4258AF: __gnu_cxx::new_allocator<std::_Rb_tree_node<std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> > > >::allocate(unsigned long, void const*) (new_allocator.h:89)
==26633==    by 0开发者_Go百科x4257A7: std::_Rb_tree<unsigned char, std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> >, std::_Select1st<std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> > >, std::less<unsigned char>, std::allocator<std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> > > >::_M_get_node() (stl_tree.h:359)
==26633==    by 0x425622: std::_Rb_tree_node<std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> > >* std::_Rb_tree<unsigned char, std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> >, std::_Select1st<std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> > >, std::less<unsigned char>, std::allocator<std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> > > >::_M_create_node<std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> > const&>(std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> > const&&&) (stl_tree.h:391)
==26633==    by 0x42526E: std::_Rb_tree<unsigned char, std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> >, std::_Select1st<std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> > >, std::less<unsigned char>, std::allocator<std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> > > >::_M_insert_(std::_Rb_tree_node_base const*, std::_Rb_tree_node_base const*, std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> > const&) (stl_tree.h:881)
==26633==    by 0x4253ED: std::_Rb_tree<unsigned char, std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> >, std::_Select1st<std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> > >, std::less<unsigned char>, std::allocator<std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> > > >::_M_insert_unique(std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> > const&) (stl_tree.h:1177)
==26633==    by 0x424CA9: std::_Rb_tree<unsigned char, std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> >, std::_Select1st<std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> > >, std::less<unsigned char>, std::allocator<std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> > > >::_M_insert_unique_(std::_Rb_tree_const_iterator<std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> > >, std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> > const&) (stl_tree.h:1217)
==26633==    by 0x424931: std::map<unsigned char, boost::shared_ptr<chat::server::network::NetworkHandler>, std::less<unsigned char>, std::allocator<std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> > > >::insert(std::_Rb_tree_iterator<std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> > >, std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> > const&) (stl_map.h:540)
==26633==    by 0x42463E: std::map<unsigned char, boost::shared_ptr<chat::server::network::NetworkHandler>, std::less<unsigned char>, std::allocator<std::pair<unsigned char const, boost::shared_ptr<chat::server::network::NetworkHandler> > > >::operator[](unsigned char const&) (stl_map.h:450)
==26633==    by 0x424163: chat::server::network::HandlerMsgHandler::addHandler(unsigned char, boost::shared_ptr<chat::server::network::NetworkHandler>) (sysnetworkhandler.cpp:19)
==26633==    by 0x4120F1: chat::server::network::UdpServer::addHandler(unsigned char, boost::shared_ptr<chat::server::network::NetworkHandler>) (udpserver.cpp:38)
==26633==    by 0x404938: chat::test::server::testCanSendToSelf(bool&) (main.cpp:95)

An std::map is a Red-Black Tree, and valgrind complains about the allocation of an std::_Rb_tree_node<std::pair<..., ...> >. The node of the map's tree is the one leaking.

I guess this cast must be related to the leak:

dynamic_cast<HandlerMsgHandler*>(sysMsgHandlers[network::handler])->addHandler(key, handler);

Are you sure that the expression sysMsgHandlers[network::handler] returns a pointer (and not a boost::shared_ptr) to HandlerMsgHandler?

EDIT: The next most probable cause is that sysMsgHandlers[handler] returns an ErrorMsgHandler instance.

EDIT, 2 Feb: Is your NetworkHandler destructor virtual? If not, the destructor of map<uint8_t boost::shared_ptr<NetworkHandler> > handlers won't get called and you'd have a leak of the map's node there.

Check out the C++ FAQ.

Q: When should my destructor be virtual?

A: When someone will delete a derived-class object via a base-class pointer. When you say delete p, and the class of p has a virtual destructor, the destructor that gets invoked is the one associated with the type of the object *p, not necessarily the one associated with the type of the pointer. This is A Good Thing.

More often than not, when I've seen a memory leak that points to memory allocated inside a container it is either that the rule of the three is not been followed and memory is leaked by the contained element, or else that the whole container is leaked.

In your particular case, it looks more like the container it self is being leaked. Something like:

int main() {
   std::map<int,std::string> *p = new std::map<int,std::string>(); // [*]
   p->insert( std::make_pair( 1, "one" ) );
}
// leaked memory allocated internally in std::map<int,std::string>

Note that the code marked with [*] need not be so obvious. The map can be a member of a class that is being dynamically allocated and never deleted.

Like vz0 said, it is the map's node that is leaking, not the handler.

Is this the only error valgrind is reporting? Valgrind report some direct leaking, so you are probably deleting/clearing the map when the program finish, but are you sure your program is not accessing out side the bound of any array or writing on uninitialized data (special care with pointers here)?

I have had a similar problem before, it happened to be a buffer overflow which was messing with the map.

Note: Dynamic cast checks at runtime if the value can be cast to that type, if it cannot, it return NULL. If you will not check if the return of dynamic_cast is NULL, then use static_cast, which is faster.