removed tricky memory leak in chat lobbies due to handling of partial messages

libretroshare/src/chat/distantchat.cc

@@ -268,6 +268,8 @@ bool DistantChatService::initiateDistantChatConnexion(const RsGxsId& to_gxs_id,
     item->PeerId(RsPeerId(tunnel_id)) ;
     handleRecvChatMsgItem(item) ;
     
+    delete item ;	// item is replaced by NULL if partial, but this is not the case here.
+    
     return true ;
 }
 

libretroshare/src/chat/distantchat.h

@@ -63,7 +63,7 @@ public:
 
     // derived in p3ChatService, so as to pass down some info
     virtual void handleIncomingItem(RsItem *) = 0;
-    virtual bool handleRecvChatMsgItem(RsChatMsgItem *ci)=0 ;
+    virtual bool handleRecvChatMsgItem(RsChatMsgItem *& ci)=0 ;
 
     bool handleOutgoingItem(RsChatItem *) ;
     bool handleRecvItem(RsChatItem *) ;

libretroshare/src/chat/distributedchat.cc

@@ -424,11 +424,6 @@ void DistributedChatService::checkSizeAndSendLobbyMessage(RsChatItem *msg)
     sendChatItem(msg) ;
 }
 
-bool DistributedChatService::locked_checkAndRebuildPartialLobbyMessage(RsChatLobbyMsgItem *ci)
-{
-    return true ;
-}
-
 bool DistributedChatService::handleRecvItem(RsChatItem *item)
 {
 	switch(item->PacketSubType())

libretroshare/src/chat/distributedchat.h

@@ -90,7 +90,6 @@ class DistributedChatService
 		void addToSaveList(std::list<RsItem*>& list) const ;
         bool processLoadListItem(const RsItem *item) ;
 
-        bool locked_checkAndRebuildPartialLobbyMessage(RsChatLobbyMsgItem *) ;
         void checkSizeAndSendLobbyMessage(RsChatItem *) ;
 
         bool sendLobbyChat(const ChatLobbyId &lobby_id, const std::string&) ;

libretroshare/src/chat/p3chatservice.cc

@@ -428,7 +428,14 @@ bool p3ChatService::sendChat(ChatId destination, std::string msg)
     return true;
 }
 
-bool p3ChatService::locked_checkAndRebuildPartialMessage(RsChatMsgItem *ci)
+// This method might take control over the memory, or modify it, possibly adding missing parts.
+// This function looks weird because it cannot duplicate the message since it does not know
+// what type of object it is and the duplicate method of lobby messages is reserved for 
+// ChatLobby  bouncing objects.
+//
+// Returns false if the item shouldn't be used (and replaced to NULL)
+
+bool p3ChatService::locked_checkAndRebuildPartialMessage(RsChatMsgItem *& ci)
 {
 	// Check is the item is ending an incomplete item.
 	//
@@ -445,13 +452,16 @@ bool p3ChatService::locked_checkAndRebuildPartialMessage(RsChatMsgItem *ci)
 
 		ci->message = it->second->message + ci->message ;
 		ci->chatFlags |= it->second->chatFlags ;
-
+        
+                // always remove existing partial. The compound message is in ci now.
+                
 		delete it->second ;
-
-		if(!ci_is_incomplete)
-			_pendingPartialMessages.erase(it) ;
+		_pendingPartialMessages.erase(it) ;
 	}
 
+        // now decide what to do: if ci is incomplete, store it and replace the pointer with NULL
+        // if complete, return it.
+        
 	if(ci_is_incomplete)
 	{
 #ifdef CHAT_DEBUG
@@ -459,7 +469,8 @@ bool p3ChatService::locked_checkAndRebuildPartialMessage(RsChatMsgItem *ci)
 #endif
 		// The item is a partial message. Push it, and wait for the rest.
 		//
-		_pendingPartialMessages[ci->PeerId()] = ci ;
+		_pendingPartialMessages[ci->PeerId()] = ci ; 	// cannot use duplicate() here
+        	ci = NULL ;					// takes memory ownership over ci
 		return false ;
 	}
 	else
@@ -503,8 +514,10 @@ void p3ChatService::handleIncomingItem(RsItem *item)
 	//
 	RsChatMsgItem *ci = dynamic_cast<RsChatMsgItem*>(item) ; 
 	if(ci != NULL) 
-    {
-		if(!  handleRecvChatMsgItem(ci))
+	{
+		handleRecvChatMsgItem(ci);
+        
+        	if(ci)
 			delete ci ;
 
 		return ;	// don't delete! It's handled by handleRecvChatMsgItem in some specific cases only.
@@ -665,7 +678,7 @@ bool p3ChatService::checkForMessageSecurity(RsChatMsgItem *ci)
 	return true ;
 }
 
-bool p3ChatService::handleRecvChatMsgItem(RsChatMsgItem *ci)
+bool p3ChatService::handleRecvChatMsgItem(RsChatMsgItem *& ci)
 {
 	time_t now = time(NULL);
 	std::string name;
@@ -674,15 +687,8 @@ bool p3ChatService::handleRecvChatMsgItem(RsChatMsgItem *ci)
     {
         RsStackMutex stack(mChatMtx); /********** STACK LOCKED MTX ******/
 
-        // This crap is because chat lobby messages use a different method for chunking messages using an additional
-        // subpacket ID, and a list of lobbies. We cannot just collapse the two because it would make the normal chat
-        // (and chat lobbies) not backward compatible.
-
-        if(!DistributedChatService::locked_checkAndRebuildPartialLobbyMessage(dynamic_cast<RsChatLobbyMsgItem*>(ci)))
-            return true ;
-
-        if(!locked_checkAndRebuildPartialMessage(ci))
-        return true ;
+        if(!locked_checkAndRebuildPartialMessage(ci))		// we make sure this call does not take control over the memory
+		return true ;					// message is a subpart of an existing message. So everything ok, but we need to return.
     }
 
     // Check for security. This avoids bombing messages, and so on.

libretroshare/src/chat/p3chatservice.h

@@ -205,7 +205,7 @@ private:
 	void receiveStateString(const RsPeerId& id,const std::string& s) ;
 
 	/// methods for handling various Chat items.
-	bool handleRecvChatMsgItem(RsChatMsgItem *item) ;			// returns false if the item should be deleted.
+	bool handleRecvChatMsgItem(RsChatMsgItem *&item) ;			// NULL-ifies the item if memory ownership is taken
 	void handleRecvChatStatusItem(RsChatStatusItem *item) ;
 	void handleRecvChatAvatarItem(RsChatAvatarItem *item) ;
 
@@ -220,7 +220,8 @@ private:
 	void checkSizeAndSendMessage(RsChatMsgItem *item) ;	// keep for compatibility for a few weeks.
 
 	/// Called when a RsChatMsgItem is received. The item may be collapsed with any waiting partial chat item from the same peer.
-	bool locked_checkAndRebuildPartialMessage(RsChatMsgItem *) ;
+        /// if so, the chat item will be turned to NULL
+	bool locked_checkAndRebuildPartialMessage(RsChatMsgItem *&) ;
 
 	RsChatAvatarItem *makeOwnAvatarItem() ;
 	RsChatStatusItem *makeOwnCustomStateStringItem() ;