From 7b2cea22d96bde8d5479437a428bc3a1fab0e05c Mon Sep 17 00:00:00 2001 From: jolavillette Date: Thu, 22 Jan 2026 15:53:51 +0100 Subject: [PATCH 1/8] implement persistence for remote peer avatars using KV sets --- src/chat/p3chatservice.cc | 246 +++++++++++++++++++++----------------- 1 file changed, 136 insertions(+), 110 deletions(-) diff --git a/src/chat/p3chatservice.cc b/src/chat/p3chatservice.cc index 9eddf9b60..b33151c88 100644 --- a/src/chat/p3chatservice.cc +++ b/src/chat/p3chatservice.cc @@ -1360,42 +1360,48 @@ void p3ChatService::setCustomStateString(const std::string& s) IndicateConfigChanged(); } -void p3ChatService::setOwnNodeAvatarData(const unsigned char *data,int size) +void p3ChatService::setOwnNodeAvatarData(const unsigned char *data, int size) { - { - RsStackMutex stack(mChatMtx); /********** STACK LOCKED MTX ******/ + RsStackMutex stack(mChatMtx); /********** STACK LOCKED MTX ******/ + + RsDbg() << "AVATAR setting own node avatar data, size: " << size; + #ifdef CHAT_DEBUG - std::cerr << "p3chatservice: Setting own avatar to new image." << std::endl ; + std::cerr << "p3chatservice: Setting own avatar to new image." << std::endl ; #endif - if((uint32_t)size > MAX_AVATAR_JPEG_SIZE) - { - std::cerr << "Supplied avatar image is too big. Maximum size is " << MAX_AVATAR_JPEG_SIZE << ", supplied image has size " << size << std::endl; - return ; - } - if(_own_avatar != NULL) - delete _own_avatar ; - - _own_avatar = new AvatarInfo(data,size) ; + /* Safety check: ensure size is positive and within RetroShare limits */ + if(size < 0 || (uint32_t)size > MAX_AVATAR_JPEG_SIZE) + { + std::cerr << "Supplied avatar image is too big or invalid. Maximum size is " << MAX_AVATAR_JPEG_SIZE << ", supplied image has size " << size << std::endl; + return ; + } - // set the info that our avatar is new, for all peers - for(std::map::iterator it(_avatars.begin());it!=_avatars.end();++it) - it->second->_own_is_new = true ; + if(_own_avatar != NULL) + { + delete _own_avatar ; } - IndicateConfigChanged(); - if(rsEvents) - { - auto e = std::make_shared(); - e->mEventCode = RsFriendListEventCode::OWN_AVATAR_CHANGED; - e->mSslId = mServiceCtrl->getOwnId(); - rsEvents->postEvent(e); - } + /* Create the new avatar info object in memory using a cast for consistency */ + _own_avatar = new AvatarInfo(data, (uint32_t)size) ; -#ifdef CHAT_DEBUG - std::cerr << "p3chatservice:setOwnAvatarJpegData() done." << std::endl ; -#endif + /* Notify the Graphical User Interface (GUI) to refresh the display */ + if(rsEvents) + { + auto e = std::make_shared(); + e->mEventCode = RsFriendListEventCode::OWN_AVATAR_CHANGED; + e->mSslId = mServiceCtrl->getOwnId(); + rsEvents->postEvent(e); + } + /* Mark own avatar as new for all connected peers to trigger network sync */ + for(std::map::iterator it(_avatars.begin()); it != _avatars.end(); ++it) + { + it->second->_own_is_new = true ; + } + + /* Indicate that configuration has changed to trigger the saveList() call at exit */ + IndicateConfigChanged(); } void p3ChatService::receiveStateString(const RsPeerId& id,const std::string& s) @@ -1420,24 +1426,21 @@ void p3ChatService::receiveStateString(const RsPeerId& id,const std::string& s) void p3ChatService::receiveAvatarJpegData(RsChatAvatarItem *ci) { - RsStackMutex stack(mChatMtx); /********** STACK LOCKED MTX ******/ -#ifdef CHAT_DEBUG - std::cerr << "p3chatservice: received avatar jpeg data for peer " << ci->PeerId() << ". Storing it." << std::endl ; -#endif + RsStackMutex stack(mChatMtx); /********** STACK LOCKED MTX ******/ + + RsDbg() << "AVATAR received data for peer " << ci->PeerId().toStdString().c_str(); - if(ci->image_size > MAX_AVATAR_JPEG_SIZE) - { - std::cerr << "Peer " << ci->PeerId()<< " is sending a jpeg image for avatar that exceeds the admitted size (size=" << ci->image_size << ", max=" << MAX_AVATAR_JPEG_SIZE << ")"<< std::endl; - return ; - } - bool new_peer = (_avatars.find(ci->PeerId()) == _avatars.end()) ; + bool new_peer = (_avatars.find(ci->PeerId()) == _avatars.end()) ; - if (new_peer == false && _avatars[ci->PeerId()]) { - delete _avatars[ci->PeerId()]; - } - _avatars[ci->PeerId()] = new AvatarInfo(ci->image_data,ci->image_size) ; - _avatars[ci->PeerId()]->_peer_is_new = true ; - _avatars[ci->PeerId()]->_own_is_new = new_peer ; + if (new_peer == false && _avatars[ci->PeerId()]) { + delete _avatars[ci->PeerId()]; + } + _avatars[ci->PeerId()] = new AvatarInfo(ci->image_data,ci->image_size) ; + _avatars[ci->PeerId()]->_peer_is_new = true ; + _avatars[ci->PeerId()]->_own_is_new = new_peer ; + + /* Force the service to save config at exit */ + IndicateConfigChanged(); } std::string p3ChatService::getOwnCustomStateString() @@ -1626,125 +1629,148 @@ std::cerr << "p3chatservice: sending requested status string for peer " << peer_ bool p3ChatService::loadList(std::list& load) { - std::list ssl_peers; - mLinkMgr->getFriendList(ssl_peers); + RsDbg() << "AVATAR entering loadList"; - for(std::list::const_iterator it(load.begin());it!=load.end();++it) + for(std::list::iterator it(load.begin()); it != load.end(); ) { - if(PrivateOugoingMapItem* om=dynamic_cast(*it)) + /* STEP 2: Handle avatars stored in KV sets (Own and Peers) */ + RsConfigKeyValueSet *kvs = dynamic_cast(*it); + if(kvs) { - RS_STACK_MUTEX(mChatMtx); - for( auto& pair : om->store ) + bool handled = false; + for(auto const& kv : kvs->tlvkvs.pairs) { - privateOutgoingMap.insert( - outMP::value_type(pair.first, - new RsChatMsgItem(pair.second)) ); - } + if(kv.key == "AV_CACHE:OWN") + { + RsDbg() << "AVATAR loading own avatar from KV set"; + std::vector decoded_data = Radix64::decode(kv.value); + if(!decoded_data.empty() && decoded_data.size() <= MAX_AVATAR_JPEG_SIZE) + { + RS_STACK_MUTEX(mChatMtx); + if(_own_avatar != NULL) delete _own_avatar; + _own_avatar = new AvatarInfo(decoded_data.data(), (uint32_t)decoded_data.size()); + } + handled = true; + } + else if(kv.key.find("AV_CACHE:") == 0) + { + std::string id_str = kv.key.substr(9); + RsPeerId pid(id_str); - delete om; continue; + if(!pid.isNull()) + { + RsDbg() << "AVATAR loading cached peer avatar for ID: " << pid.toStdString().c_str(); + std::vector decoded_data = Radix64::decode(kv.value); + + if(!decoded_data.empty() && decoded_data.size() <= MAX_AVATAR_JPEG_SIZE) + { + RS_STACK_MUTEX(mChatMtx); + if (_avatars.count(pid)) delete _avatars[pid]; + _avatars[pid] = new AvatarInfo(decoded_data.data(), (uint32_t)decoded_data.size()); + } + } + handled = true; + } + } + if(handled) { + delete *it; + it = load.erase(it); + continue; + } } - - RsChatAvatarItem *ai = NULL ; - - if(NULL != (ai = dynamic_cast(*it))) + /* Legacy avatar handling: we skip it for peers but keep it for cleanup */ + RsChatAvatarItem *ai = dynamic_cast(*it); + if(ai) { - RS_STACK_MUTEX(mChatMtx); - - if(ai->image_size <= MAX_AVATAR_JPEG_SIZE) - _own_avatar = new AvatarInfo(ai->image_data,ai->image_size) ; - else - std::cerr << "Dropping avatar image, because its size is " - << ai->image_size << ", and the maximum allowed size " - << "is " << MAX_AVATAR_JPEG_SIZE << std::endl; - delete *it; + it = load.erase(it); continue; } - RsChatStatusItem *mitem = NULL ; - - if(NULL != (mitem = dynamic_cast(*it))) + /* Original developer code: Handle custom status string */ + RsChatStatusItem *mitem = dynamic_cast(*it); + if(mitem) { - RS_STACK_MUTEX(mChatMtx); - _custom_status_string = mitem->status_string ; - delete *it; + it = load.erase(it); continue; } - /* TODO: G10h4ck 2017/02/27 this block is kept for retrocompatibility, - * and will be used just first time, to load messages in the old format - * should be removed in the following RS version */ - if( RsPrivateChatMsgConfigItem *citem = - dynamic_cast(*it) ) + /* Original developer code: Handle saved outgoing messages */ + PrivateOugoingMapItem* om = dynamic_cast(*it); + if(om) { RS_STACK_MUTEX(mChatMtx); - - if ( citem->chatFlags & RS_CHAT_FLAG_PRIVATE ) + for( auto& pair : om->store ) { - if ( std::find(ssl_peers.begin(), ssl_peers.end(), - citem->configPeerId) != ssl_peers.end() ) - { - RsChatMsgItem *ci = new RsChatMsgItem(); - citem->get(ci); - - if (citem->configFlags & RS_CHATMSG_CONFIGFLAG_INCOMING) - { - locked_storeIncomingMsg(ci); - } - else privateOutgoingMap.insert( - outMP::value_type(RSRandom::random_u64(), ci) ); - } - else { /* no friends */ } + privateOutgoingMap.insert( + outMP::value_type(pair.first, + new RsChatMsgItem(pair.second)) ); } - else { /* ignore all other items */ } - delete *it; + it = load.erase(it); continue; } - DistributedChatService::processLoadListItem(*it) ; - DistantChatService::processLoadListItem(*it) ; - - // delete unknown items - delete *it; + ++it; } - load.clear() ; - return true; + return true ; } bool p3ChatService::saveList(bool& cleanup, std::list& list) { cleanup = true; - /* now we create a pqistore, and stream all the msgs into it */ + RsDbg() << "AVATAR entering saveList"; - if(_own_avatar != NULL) + /* Save own avatar using KV Set for better reliability */ + if(_own_avatar != NULL && _own_avatar->_image_size > 0) { - RsChatAvatarItem *ci = makeOwnAvatarItem() ; - ci->PeerId(mServiceCtrl->getOwnId()); + RsDbg() << "AVATAR saving own avatar in KV set for peer: " << mServiceCtrl->getOwnId().toStdString().c_str(); + + RsConfigKeyValueSet *avatar_item = new RsConfigKeyValueSet(); + RsTlvKeyValue kv; + kv.key = "AV_CACHE:OWN"; + Radix64::encode(_own_avatar->_image_data, _own_avatar->_image_size, kv.value); + avatar_item->tlvkvs.pairs.push_back(kv); + list.push_back(avatar_item); + } - list.push_back(ci) ; + /* STEP 1: Loop through cached peer avatars and save them using KV sets */ + for(auto const& it : _avatars) + { + if (it.second != nullptr && it.second->_image_size > 0 && !it.first.isNull()) + { + RsDbg() << "AVATAR saving peer avatar in KV set for peer: " << it.first.toStdString().c_str(); + + RsConfigKeyValueSet *avatar_item = new RsConfigKeyValueSet(); + RsTlvKeyValue kv; + kv.key = "AV_CACHE:" + it.first.toStdString(); + Radix64::encode(it.second->_image_data, it.second->_image_size, kv.value); + avatar_item->tlvkvs.pairs.push_back(kv); + list.push_back(avatar_item); + } } mChatMtx.lock(); /****** MUTEX LOCKED *******/ + /* Original developer code: Save outgoing messages waiting for peer connection */ PrivateOugoingMapItem* om = new PrivateOugoingMapItem; typedef std::map::value_type vT; for( auto& pair : privateOutgoingMap ) om->store.insert(vT(pair.first, *pair.second)); list.push_back(om); - + /* Original developer code: Save custom status string */ RsChatStatusItem *di = new RsChatStatusItem ; di->status_string = _custom_status_string ; di->flags = RS_CHAT_FLAG_CUSTOM_STATE ; - list.push_back(di); + /* Mandatory calls to parent services for their own data persistence */ DistributedChatService::addToSaveList(list) ; DistantChatService::addToSaveList(list) ; From f4fb17895b96cb1594ba20d183611d7404426337 Mon Sep 17 00:00:00 2001 From: jolavillette Date: Thu, 22 Jan 2026 16:17:59 +0100 Subject: [PATCH 2/8] initiate proactive avatar exchange on peer connection --- src/chat/p3chatservice.cc | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/chat/p3chatservice.cc b/src/chat/p3chatservice.cc index b33151c88..b35351554 100644 --- a/src/chat/p3chatservice.cc +++ b/src/chat/p3chatservice.cc @@ -1793,7 +1793,6 @@ RsSerialiser *p3ChatService::setupSerialiser() } /*************** pqiMonitor callback ***********************/ - void p3ChatService::statusChange(const std::list &plist) { for (auto it = plist.cbegin(); it != plist.cend(); ++it) @@ -1843,6 +1842,25 @@ void p3ChatService::statusChange(const std::list &plist) if (changed) IndicateConfigChanged(); + + /* Mandatory avatar exchange on connection to handle offline changes */ + RsDbg() << "AVATAR peer connected, initiating exchange with: " << it->id.toStdString().c_str(); + + // 1. Request the peer's avatar to check for updates while we were offline + sendAvatarRequest(it->id); + + // 2. Notify the peer that our own avatar is available + if(_own_avatar != nullptr && _own_avatar->_image_size > 0) + { + RsChatMsgItem *nav = new RsChatMsgItem(); + nav->PeerId(it->id); + + // Signal that our avatar is available without sending the full image yet + nav->chatFlags = RS_CHAT_FLAG_PRIVATE | RS_CHAT_FLAG_AVATAR_AVAILABLE; + nav->sendTime = time(NULL); + + sendChatItem(nav); + } } else if (it->actions & RS_SERVICE_PEER_REMOVED) { @@ -1861,3 +1879,4 @@ void p3ChatService::statusChange(const std::list &plist) } } } + From da86348a5356b007ceb919b497e0d8c54bebabae Mon Sep 17 00:00:00 2001 From: jolavillette Date: Thu, 22 Jan 2026 16:59:18 +0100 Subject: [PATCH 3/8] implement direct avatar broadcast on update and on connection --- src/chat/p3chatservice.cc | 97 ++++++++++++++++++++++----------------- 1 file changed, 55 insertions(+), 42 deletions(-) diff --git a/src/chat/p3chatservice.cc b/src/chat/p3chatservice.cc index b35351554..c1b11f63d 100644 --- a/src/chat/p3chatservice.cc +++ b/src/chat/p3chatservice.cc @@ -1362,45 +1362,62 @@ void p3ChatService::setCustomStateString(const std::string& s) void p3ChatService::setOwnNodeAvatarData(const unsigned char *data, int size) { - RsStackMutex stack(mChatMtx); /********** STACK LOCKED MTX ******/ + std::set onlineList; + + { + /* Use a limited scope for the mutex to avoid deadlocks during broadcast */ + RsStackMutex stack(mChatMtx); /********** STACK LOCKED MTX ******/ - RsDbg() << "AVATAR setting own node avatar data, size: " << size; + RsDbg() << "AVATAR setting own node avatar data, size: " << size; #ifdef CHAT_DEBUG - std::cerr << "p3chatservice: Setting own avatar to new image." << std::endl ; + std::cerr << "p3chatservice: Setting own avatar to new image." << std::endl ; #endif - /* Safety check: ensure size is positive and within RetroShare limits */ - if(size < 0 || (uint32_t)size > MAX_AVATAR_JPEG_SIZE) - { - std::cerr << "Supplied avatar image is too big or invalid. Maximum size is " << MAX_AVATAR_JPEG_SIZE << ", supplied image has size " << size << std::endl; - return ; - } + /* Safety check: ensure size is positive and within RetroShare limits */ + if(size < 0 || (uint32_t)size > MAX_AVATAR_JPEG_SIZE) + { + std::cerr << "Supplied avatar image is too big or invalid. Maximum size is " << MAX_AVATAR_JPEG_SIZE << ", supplied image has size " << size << std::endl; + return ; + } - if(_own_avatar != NULL) - { - delete _own_avatar ; - } + if(_own_avatar != NULL) + { + delete _own_avatar ; + } - /* Create the new avatar info object in memory using a cast for consistency */ - _own_avatar = new AvatarInfo(data, (uint32_t)size) ; + /* Create the new avatar info object in memory */ + _own_avatar = new AvatarInfo(data, (uint32_t)size) ; - /* Notify the Graphical User Interface (GUI) to refresh the display */ - if(rsEvents) - { - auto e = std::make_shared(); - e->mEventCode = RsFriendListEventCode::OWN_AVATAR_CHANGED; - e->mSslId = mServiceCtrl->getOwnId(); - rsEvents->postEvent(e); - } + /* Notify the Graphical User Interface (GUI) to refresh the display */ + if(rsEvents) + { + auto e = std::make_shared(); + e->mEventCode = RsFriendListEventCode::OWN_AVATAR_CHANGED; + e->mSslId = mServiceCtrl->getOwnId(); + rsEvents->postEvent(e); + } - /* Mark own avatar as new for all connected peers to trigger network sync */ - for(std::map::iterator it(_avatars.begin()); it != _avatars.end(); ++it) + /* Mark own avatar as new for all cached peers for internal consistency */ + for(std::map::iterator it(_avatars.begin()); it != _avatars.end(); ++it) + { + it->second->_own_is_new = true ; + } + + /* Get the list of currently online peers while the mutex is still held */ + mServiceCtrl->getPeersConnected(getServiceInfo().mServiceType, onlineList); + + } /* mChatMtx is automatically released here when 'stack' goes out of scope */ + + /* BROADCAST: Now safely send the avatar to all online peers without deadlock. + * sendAvatarJpegData will acquire the mutex internally via makeOwnAvatarItem. */ + for(std::set::iterator it = onlineList.begin(); it != onlineList.end(); ++it) { - it->second->_own_is_new = true ; + RsDbg() << "AVATAR broadcasting image data to peer: " << it->toStdString().c_str(); + sendAvatarJpegData(*it); } - /* Indicate that configuration has changed to trigger the saveList() call at exit */ + /* Trigger configuration save to persist the new avatar to disk */ IndicateConfigChanged(); } @@ -1793,6 +1810,7 @@ RsSerialiser *p3ChatService::setupSerialiser() } /*************** pqiMonitor callback ***********************/ + void p3ChatService::statusChange(const std::list &plist) { for (auto it = plist.cbegin(); it != plist.cend(); ++it) @@ -1805,7 +1823,9 @@ void p3ChatService::statusChange(const std::list &plist) std::vector to_send; { - RS_STACK_MUTEX(mChatMtx); + /* The mutex is scoped here in original code, which prevents deadlocks + * with the subsequent avatar sync calls below. */ + RS_STACK_MUTEX(mChatMtx); /********** STACK LOCKED MTX ******/ for( auto cit = privateOutgoingMap.begin(); cit != privateOutgoingMap.end(); ) @@ -1823,7 +1843,7 @@ void p3ChatService::statusChange(const std::list &plist) ++cit; } - } + } /* Lock is released here */ for(auto toIt = to_send.begin(); toIt != to_send.end(); ++toIt) { @@ -1843,23 +1863,17 @@ void p3ChatService::statusChange(const std::list &plist) if (changed) IndicateConfigChanged(); - /* Mandatory avatar exchange on connection to handle offline changes */ - RsDbg() << "AVATAR peer connected, initiating exchange with: " << it->id.toStdString().c_str(); + /* AVATAR: Proactive direct broadcast on connection to sync potential offline changes */ + RsDbg() << "AVATAR peer connected, initiating direct exchange with: " << it->id.toStdString().c_str(); - // 1. Request the peer's avatar to check for updates while we were offline + // 1. Proactively request the peer's avatar sendAvatarRequest(it->id); - // 2. Notify the peer that our own avatar is available + // 2. Directly push our own avatar data to the connecting peer. + // Safety: This is called outside the mutex block to avoid recursive locking deadlock. if(_own_avatar != nullptr && _own_avatar->_image_size > 0) { - RsChatMsgItem *nav = new RsChatMsgItem(); - nav->PeerId(it->id); - - // Signal that our avatar is available without sending the full image yet - nav->chatFlags = RS_CHAT_FLAG_PRIVATE | RS_CHAT_FLAG_AVATAR_AVAILABLE; - nav->sendTime = time(NULL); - - sendChatItem(nav); + sendAvatarJpegData(it->id); } } else if (it->actions & RS_SERVICE_PEER_REMOVED) @@ -1879,4 +1893,3 @@ void p3ChatService::statusChange(const std::list &plist) } } } - From 19af04e5ab949e532b1166a9b84c5828874781e0 Mon Sep 17 00:00:00 2001 From: jolavillette Date: Thu, 22 Jan 2026 18:19:16 +0100 Subject: [PATCH 4/8] restore lobby persistence via parent delegation and fix recursive deadlock --- src/chat/p3chatservice.cc | 196 ++++++++++++++++---------------------- 1 file changed, 84 insertions(+), 112 deletions(-) diff --git a/src/chat/p3chatservice.cc b/src/chat/p3chatservice.cc index c1b11f63d..1e173a66e 100644 --- a/src/chat/p3chatservice.cc +++ b/src/chat/p3chatservice.cc @@ -1365,7 +1365,8 @@ void p3ChatService::setOwnNodeAvatarData(const unsigned char *data, int size) std::set onlineList; { - /* Use a limited scope for the mutex to avoid deadlocks during broadcast */ + /* We use a scoped block to release the mutex before broadcasting, + * preventing the deadlock you saw in GDB. */ RsStackMutex stack(mChatMtx); /********** STACK LOCKED MTX ******/ RsDbg() << "AVATAR setting own node avatar data, size: " << size; @@ -1374,22 +1375,17 @@ void p3ChatService::setOwnNodeAvatarData(const unsigned char *data, int size) std::cerr << "p3chatservice: Setting own avatar to new image." << std::endl ; #endif - /* Safety check: ensure size is positive and within RetroShare limits */ if(size < 0 || (uint32_t)size > MAX_AVATAR_JPEG_SIZE) { - std::cerr << "Supplied avatar image is too big or invalid. Maximum size is " << MAX_AVATAR_JPEG_SIZE << ", supplied image has size " << size << std::endl; + std::cerr << "Supplied avatar image is too big. Max is " << MAX_AVATAR_JPEG_SIZE << std::endl; return ; } if(_own_avatar != NULL) - { delete _own_avatar ; - } - /* Create the new avatar info object in memory */ - _own_avatar = new AvatarInfo(data, (uint32_t)size) ; + _own_avatar = new AvatarInfo(data,(uint32_t)size) ; - /* Notify the Graphical User Interface (GUI) to refresh the display */ if(rsEvents) { auto e = std::make_shared(); @@ -1398,26 +1394,18 @@ void p3ChatService::setOwnNodeAvatarData(const unsigned char *data, int size) rsEvents->postEvent(e); } - /* Mark own avatar as new for all cached peers for internal consistency */ - for(std::map::iterator it(_avatars.begin()); it != _avatars.end(); ++it) - { + for(std::map::iterator it(_avatars.begin());it!=_avatars.end();++it) it->second->_own_is_new = true ; - } - /* Get the list of currently online peers while the mutex is still held */ mServiceCtrl->getPeersConnected(getServiceInfo().mServiceType, onlineList); + } - } /* mChatMtx is automatically released here when 'stack' goes out of scope */ - - /* BROADCAST: Now safely send the avatar to all online peers without deadlock. - * sendAvatarJpegData will acquire the mutex internally via makeOwnAvatarItem. */ for(std::set::iterator it = onlineList.begin(); it != onlineList.end(); ++it) { - RsDbg() << "AVATAR broadcasting image data to peer: " << it->toStdString().c_str(); + RsDbg() << "AVATAR broadcasting to peer: " << it->toStdString().c_str(); sendAvatarJpegData(*it); } - /* Trigger configuration save to persist the new avatar to disk */ IndicateConfigChanged(); } @@ -1596,15 +1584,17 @@ RsChatStatusItem *p3ChatService::makeOwnCustomStateStringItem() RsChatAvatarItem *p3ChatService::makeOwnAvatarItem() { - RsStackMutex stack(mChatMtx); /********** STACK LOCKED MTX ******/ + /* Mutex is removed here to prevent recursive deadlock from parent calls */ RsChatAvatarItem *ci = new RsChatAvatarItem(); - _own_avatar->toUnsignedChar(ci->image_data,ci->image_size) ; + if(_own_avatar != nullptr) + { + _own_avatar->toUnsignedChar(ci->image_data,ci->image_size) ; + } return ci ; } - void p3ChatService::sendAvatarJpegData(const RsPeerId& peer_id) { #ifdef CHAT_DEBUG @@ -1646,92 +1636,98 @@ std::cerr << "p3chatservice: sending requested status string for peer " << peer_ bool p3ChatService::loadList(std::list& load) { - RsDbg() << "AVATAR entering loadList"; + RsDbg() << "AVATAR entering loadList, total items: " << (int)load.size(); for(std::list::iterator it(load.begin()); it != load.end(); ) { - /* STEP 2: Handle avatars stored in KV sets (Own and Peers) */ - RsConfigKeyValueSet *kvs = dynamic_cast(*it); + bool item_handled = false; + RsItem *item = *it; + + /* 1. Attempt to handle Avatar data (local p3ChatService logic) */ + RsConfigKeyValueSet *kvs = dynamic_cast(item); if(kvs) { - bool handled = false; + /* Fixed: Using 'kv' to match the loop variable name */ for(auto const& kv : kvs->tlvkvs.pairs) { if(kv.key == "AV_CACHE:OWN") { - RsDbg() << "AVATAR loading own avatar from KV set"; std::vector decoded_data = Radix64::decode(kv.value); if(!decoded_data.empty() && decoded_data.size() <= MAX_AVATAR_JPEG_SIZE) { RS_STACK_MUTEX(mChatMtx); if(_own_avatar != NULL) delete _own_avatar; _own_avatar = new AvatarInfo(decoded_data.data(), (uint32_t)decoded_data.size()); + item_handled = true; } - handled = true; } else if(kv.key.find("AV_CACHE:") == 0) { - std::string id_str = kv.key.substr(9); - RsPeerId pid(id_str); - + RsPeerId pid(kv.key.substr(9)); if(!pid.isNull()) { - RsDbg() << "AVATAR loading cached peer avatar for ID: " << pid.toStdString().c_str(); std::vector decoded_data = Radix64::decode(kv.value); - if(!decoded_data.empty() && decoded_data.size() <= MAX_AVATAR_JPEG_SIZE) { RS_STACK_MUTEX(mChatMtx); if (_avatars.count(pid)) delete _avatars[pid]; _avatars[pid] = new AvatarInfo(decoded_data.data(), (uint32_t)decoded_data.size()); + item_handled = true; } } - handled = true; } } - if(handled) { - delete *it; - it = load.erase(it); - continue; - } } - /* Legacy avatar handling: we skip it for peers but keep it for cleanup */ - RsChatAvatarItem *ai = dynamic_cast(*it); - if(ai) + /* 2. Handle original Chat Service items (Status & Outgoing) */ + if (!item_handled) { - delete *it; - it = load.erase(it); - continue; + RsChatStatusItem *mitem = dynamic_cast(item); + if(mitem) + { + _custom_status_string = mitem->status_string ; + item_handled = true; + } + + PrivateOugoingMapItem* om = dynamic_cast(item); + if(om) + { + RS_STACK_MUTEX(mChatMtx); + for( auto& pair : om->store ) + privateOutgoingMap.insert(outMP::value_type(pair.first, new RsChatMsgItem(pair.second))); + item_handled = true; + } } - /* Original developer code: Handle custom status string */ - RsChatStatusItem *mitem = dynamic_cast(*it); - if(mitem) + /* 3. THE RELAY: This is what restores your lobbies. + * If we haven't handled the item, we ask the parents if they recognize it. */ + if (!item_handled) { - _custom_status_string = mitem->status_string ; - delete *it; - it = load.erase(it); - continue; + if (DistributedChatService::processLoadListItem(item)) + { + item_handled = true; + } } - /* Original developer code: Handle saved outgoing messages */ - PrivateOugoingMapItem* om = dynamic_cast(*it); - if(om) + if (!item_handled) { - RS_STACK_MUTEX(mChatMtx); - for( auto& pair : om->store ) + if (DistantChatService::processLoadListItem(item)) { - privateOutgoingMap.insert( - outMP::value_type(pair.first, - new RsChatMsgItem(pair.second)) ); + item_handled = true; } - delete *it; - it = load.erase(it); - continue; } - ++it; + /* 4. Final Decision: Only remove items that were successfully processed */ + if(item_handled) + { + delete item; + it = load.erase(it); + } + else + { + /* If unhandled, move to the next item so other services can see it */ + ++it; + } } return true ; @@ -1741,53 +1737,42 @@ bool p3ChatService::saveList(bool& cleanup, std::list& list) { cleanup = true; - RsDbg() << "AVATAR entering saveList"; - - /* Save own avatar using KV Set for better reliability */ + /* 1. Save avatars */ if(_own_avatar != NULL && _own_avatar->_image_size > 0) { - RsDbg() << "AVATAR saving own avatar in KV set for peer: " << mServiceCtrl->getOwnId().toStdString().c_str(); - - RsConfigKeyValueSet *avatar_item = new RsConfigKeyValueSet(); - RsTlvKeyValue kv; - kv.key = "AV_CACHE:OWN"; + RsConfigKeyValueSet *item = new RsConfigKeyValueSet(); + RsTlvKeyValue kv; kv.key = "AV_CACHE:OWN"; Radix64::encode(_own_avatar->_image_data, _own_avatar->_image_size, kv.value); - avatar_item->tlvkvs.pairs.push_back(kv); - list.push_back(avatar_item); + item->tlvkvs.pairs.push_back(kv); + list.push_back(item); } - /* STEP 1: Loop through cached peer avatars and save them using KV sets */ - for(auto const& it : _avatars) + for(auto const& [pid, info] : _avatars) { - if (it.second != nullptr && it.second->_image_size > 0 && !it.first.isNull()) + if (info != NULL && info->_image_size > 0 && !pid.isNull()) { - RsDbg() << "AVATAR saving peer avatar in KV set for peer: " << it.first.toStdString().c_str(); - - RsConfigKeyValueSet *avatar_item = new RsConfigKeyValueSet(); - RsTlvKeyValue kv; - kv.key = "AV_CACHE:" + it.first.toStdString(); - Radix64::encode(it.second->_image_data, it.second->_image_size, kv.value); - avatar_item->tlvkvs.pairs.push_back(kv); - list.push_back(avatar_item); + RsConfigKeyValueSet *item = new RsConfigKeyValueSet(); + RsTlvKeyValue kv; kv.key = "AV_CACHE:" + pid.toStdString(); + Radix64::encode(info->_image_data, info->_image_size, kv.value); + item->tlvkvs.pairs.push_back(kv); + list.push_back(item); } } - mChatMtx.lock(); /****** MUTEX LOCKED *******/ + /* 2. Original developer items (Messages & Status) */ + mChatMtx.lock(); - /* Original developer code: Save outgoing messages waiting for peer connection */ PrivateOugoingMapItem* om = new PrivateOugoingMapItem; - typedef std::map::value_type vT; for( auto& pair : privateOutgoingMap ) - om->store.insert(vT(pair.first, *pair.second)); + om->store.insert(std::map::value_type(pair.first, *pair.second)); list.push_back(om); - /* Original developer code: Save custom status string */ RsChatStatusItem *di = new RsChatStatusItem ; di->status_string = _custom_status_string ; di->flags = RS_CHAT_FLAG_CUSTOM_STATE ; list.push_back(di); - /* Mandatory calls to parent services for their own data persistence */ + /* 3. CRITICAL: Save Chat Lobbies and Distant Chat subscriptions */ DistributedChatService::addToSaveList(list) ; DistantChatService::addToSaveList(list) ; @@ -1815,35 +1800,28 @@ void p3ChatService::statusChange(const std::list &plist) { for (auto it = plist.cbegin(); it != plist.cend(); ++it) { - if (it->actions & RS_SERVICE_PEER_CONNECTED) + if (it->actions & RS_SERVICE_PEER_CONNECTED) { /* send the saved outgoing messages */ bool changed = false; - std::vector to_send; { - /* The mutex is scoped here in original code, which prevents deadlocks - * with the subsequent avatar sync calls below. */ - RS_STACK_MUTEX(mChatMtx); /********** STACK LOCKED MTX ******/ - - for( auto cit = privateOutgoingMap.begin(); - cit != privateOutgoingMap.end(); ) + RS_STACK_MUTEX(mChatMtx); + for( auto cit = privateOutgoingMap.begin(); cit != privateOutgoingMap.end(); ) { RsChatMsgItem *c = cit->second; if (c->PeerId() == it->id) { //mHistoryMgr->addMessage(false, c->PeerId(), ownId, c); - to_send.push_back(c) ; changed = true; cit = privateOutgoingMap.erase(cit); continue; } - ++cit; } - } /* Lock is released here */ + } for(auto toIt = to_send.begin(); toIt != to_send.end(); ++toIt) { @@ -1863,27 +1841,21 @@ void p3ChatService::statusChange(const std::list &plist) if (changed) IndicateConfigChanged(); - /* AVATAR: Proactive direct broadcast on connection to sync potential offline changes */ - RsDbg() << "AVATAR peer connected, initiating direct exchange with: " << it->id.toStdString().c_str(); - - // 1. Proactively request the peer's avatar + /* AVATAR Handshake on connection */ + RsDbg() << "AVATAR peer connected, initiating sync with: " << it->id.toStdString().c_str(); sendAvatarRequest(it->id); - - // 2. Directly push our own avatar data to the connecting peer. - // Safety: This is called outside the mutex block to avoid recursive locking deadlock. if(_own_avatar != nullptr && _own_avatar->_image_size > 0) { sendAvatarJpegData(it->id); } } - else if (it->actions & RS_SERVICE_PEER_REMOVED) + else if (it->actions & RS_SERVICE_PEER_REMOVED) { /* now handle remove */ mHistoryMgr->clear(ChatId(it->id)); RS_STACK_MUTEX(mChatMtx); - for ( auto cit = privateOutgoingMap.begin(); - cit != privateOutgoingMap.end(); ) + for ( auto cit = privateOutgoingMap.begin(); cit != privateOutgoingMap.end(); ) { RsChatMsgItem *c = cit->second; if (c->PeerId() == it->id) cit = privateOutgoingMap.erase(cit); From 2e73cfd7a46e6c4d157f3eff87badd459434a127 Mon Sep 17 00:00:00 2001 From: jolavillette Date: Sat, 24 Jan 2026 17:50:54 +0100 Subject: [PATCH 5/8] code and logic improvement following cyril comment --- src/chat/p3chatservice.cc | 457 ++++++++++++++++++++------------------ src/chat/p3chatservice.h | 4 +- 2 files changed, 245 insertions(+), 216 deletions(-) diff --git a/src/chat/p3chatservice.cc b/src/chat/p3chatservice.cc index 1e173a66e..f3b751331 100644 --- a/src/chat/p3chatservice.cc +++ b/src/chat/p3chatservice.cc @@ -43,9 +43,7 @@ #include "chat/p3chatservice.h" #include "rsitems/rsconfigitems.h" -/**** - * #define CHAT_DEBUG 1 - ****/ +#define CHAT_DEBUG 1 RsChats *rsChats = nullptr; @@ -356,66 +354,66 @@ void p3ChatService::sendPublicChat(const std::string &msg) } } - +/* Inside p3ChatService class definition */ class p3ChatService::AvatarInfo { - public: - AvatarInfo() - { - _image_size = 0 ; - _image_data = NULL ; - _peer_is_new = false ; // true when the peer has a new avatar - _own_is_new = false ; // true when I myself a new avatar to send to this peer. - } - - ~AvatarInfo() - { - free( _image_data ); - _image_data = NULL ; - _image_size = 0 ; - } - - AvatarInfo(const AvatarInfo& ai) - { - init(ai._image_data,ai._image_size) ; - } - - void init(const unsigned char *jpeg_data,int size) - { - if(size == 0) - { - _image_size = 0; - _image_data = nullptr; - } - else - { - _image_size = size ; - _image_data = (unsigned char*)rs_malloc(size) ; - memcpy(_image_data,jpeg_data,size) ; - } - } - AvatarInfo(const unsigned char *jpeg_data,int size) - { - init(jpeg_data,size) ; - } - - void toUnsignedChar(unsigned char *& data,uint32_t& size) const - { - if(_image_size == 0) - { - size = 0 ; - data = NULL ; - return ; - } - data = (unsigned char *)rs_malloc(_image_size) ; - size = _image_size ; - memcpy(data,_image_data,size*sizeof(unsigned char)) ; - } - - uint32_t _image_size ; - unsigned char *_image_data ; - int _peer_is_new ; // true when the peer has a new avatar - int _own_is_new ; // true when I myself a new avatar to send to this peer. +public: + /* Fix: Initialize in the exact order of declaration (_image_size then _image_data) */ + AvatarInfo() : _image_size(0), _image_data(NULL), _peer_is_new(false), _own_is_new(false), _last_request_time(0) {} + + ~AvatarInfo() { if (_image_data) free(_image_data); } + + /* Fix: Constructor for Radix64 loading (Peer Storage) */ + AvatarInfo(const std::string& r64_data) : _image_size(0), _image_data(NULL) + { + if (!r64_data.empty()) + { + /* Use the vector-based decode required by your SDK */ + std::vector tmp = Radix64::decode(r64_data); + if (!tmp.empty()) { + init(tmp.data(), (int)tmp.size()); + } + } + _peer_is_new = false; + _own_is_new = false; + _last_request_time = 0; + } + + /* Fix: Use the 3-argument encode required by your SDK */ + std::string toRadix64() const + { + std::string out; + if (_image_data && _image_size > 0) { + Radix64::encode(_image_data, (size_t)_image_size, out); + } + return out; + } + + void init(const unsigned char *jpeg_data, int size) + { + if (size > 0) { + _image_size = size; + _image_data = (unsigned char*)rs_malloc(size); + memcpy(_image_data, jpeg_data, size); + } + } + + /* Order must be identical here too */ + AvatarInfo(const unsigned char *jpeg_data, int size) : _image_size(0), _image_data(NULL) { init(jpeg_data, size); } + + void toUnsignedChar(unsigned char *& data, uint32_t& size) const + { + if (_image_size == 0) { size = 0; data = NULL; return; } + data = (unsigned char *)rs_malloc(_image_size); + size = _image_size; + memcpy(data, _image_data, size); + } + + uint32_t _image_size; + unsigned char *_image_data; + bool _peer_is_new; + bool _own_is_new; + time_t _last_request_time; }; void p3ChatService::sendGroupChatStatusString(const std::string& status_string) @@ -782,7 +780,9 @@ bool p3ChatService::sendChat(ChatId destination, std::string msg) #ifdef CHAT_DEBUG std::cerr << "own status string is new for peer " << vpid << ": sending it." << std::endl ; #endif - RsChatStatusItem *cs = makeOwnCustomStateStringItem() ; + /* Lock specifically to access the shared status string safely */ + RS_STACK_MUTEX(mChatMtx); + RsChatStatusItem *cs = locked_makeOwnCustomStateStringItem() ; cs->PeerId(vpid) ; sendChatItem(cs) ; } @@ -1182,7 +1182,7 @@ bool p3ChatService::handleRecvChatMsgItem(RsChatMsgItem *& ci) std::map::const_iterator it = _avatars.find(ci->PeerId()) ; #ifdef CHAT_DEBUG - std::cerr << "p3chatservice:: avatar requested from above. " << std::endl ; + // std::cerr << "p3chatservice:: avatar requested from above. " << std::endl ; #endif // has avatar. Return it strait away. // @@ -1431,21 +1431,22 @@ void p3ChatService::receiveStateString(const RsPeerId& id,const std::string& s) void p3ChatService::receiveAvatarJpegData(RsChatAvatarItem *ci) { - RsStackMutex stack(mChatMtx); /********** STACK LOCKED MTX ******/ - - RsDbg() << "AVATAR received data for peer " << ci->PeerId().toStdString().c_str(); + RsPeerId pid = ci->PeerId(); + RsPeerId ownId = mServiceCtrl->getOwnId(); - bool new_peer = (_avatars.find(ci->PeerId()) == _avatars.end()) ; + /* Safety: Do not let network packets overwrite local 'Self' data */ + if(pid.isNull() || (!ownId.isNull() && pid == ownId)) { + RsDbg() << "AVATAR: [RECV] Ignored incoming avatar packet identifying as SELF."; + return; + } - if (new_peer == false && _avatars[ci->PeerId()]) { - delete _avatars[ci->PeerId()]; - } - _avatars[ci->PeerId()] = new AvatarInfo(ci->image_data,ci->image_size) ; - _avatars[ci->PeerId()]->_peer_is_new = true ; - _avatars[ci->PeerId()]->_own_is_new = new_peer ; + RS_STACK_MUTEX(mChatMtx); + RsDbg() << "AVATAR: [RECV] Received valid avatar for peer: " << pid.toStdString(); + if (_avatars.count(pid)) delete _avatars[pid]; + _avatars[pid] = new AvatarInfo(ci->image_data, ci->image_size); + _avatars[pid]->_peer_is_new = true; - /* Force the service to save config at exit */ - IndicateConfigChanged(); + IndicateConfigChanged(); } std::string p3ChatService::getOwnCustomStateString() @@ -1460,7 +1461,7 @@ void p3ChatService::getOwnNodeAvatarData(unsigned char *& data,int& size) uint32_t s = 0 ; #ifdef CHAT_DEBUG - std::cerr << "p3chatservice:: own avatar requested from above. " << std::endl ; + //std::cerr << "p3chatservice:: own avatar requested from above. " << std::endl ; #endif // has avatar. Return it strait away. // @@ -1506,7 +1507,7 @@ void p3ChatService::getAvatarData(const RsPeerId& peer_id,unsigned char *& data, std::map::const_iterator it = _avatars.find(peer_id) ; #ifdef CHAT_DEBUG - std::cerr << "p3chatservice:: avatar for peer " << peer_id << " requested from above. " << std::endl ; + //std::cerr << "p3chatservice:: avatar for peer " << peer_id << " requested from above. " << std::endl ; #endif // has avatar. Return it straight away. // @@ -1521,13 +1522,22 @@ void p3ChatService::getAvatarData(const RsPeerId& peer_id,unsigned char *& data, #endif return ; } else { + /* Create placeholder to store request time */ + if (it == _avatars.end()) { + _avatars[peer_id] = new AvatarInfo(); + it = _avatars.find(peer_id); + } + + time_t now = time(NULL); + if (now - it->second->_last_request_time > 60) { #ifdef CHAT_DEBUG - std::cerr << "No avatar for this peer. Requesting it by sending request packet." << std::endl ; + RsDbg() << "AVATAR p3ChatService::getAvatarData: No avatar for peer " << peer_id << ". Requesting it (throttled)."; #endif + it->second->_last_request_time = now; + sendAvatarRequest(peer_id); + } } } - - sendAvatarRequest(peer_id); } void p3ChatService::sendAvatarRequest(const RsPeerId& peer_id) @@ -1545,8 +1555,7 @@ void p3ChatService::sendAvatarRequest(const RsPeerId& peer_id) ci->message.erase(); #ifdef CHAT_DEBUG - std::cerr << "p3ChatService::sending request for avatar, to peer " << peer_id << std::endl ; - std::cerr << std::endl; + RsDbg() << "AVATAR p3ChatService::sendAvatarRequest: sending request for avatar to peer " << peer_id; #endif sendChatItem(ci); @@ -1571,9 +1580,10 @@ void p3ChatService::sendCustomStateRequest(const RsPeerId& peer_id){ sendChatItem(cs); } -RsChatStatusItem *p3ChatService::makeOwnCustomStateStringItem() +RsChatStatusItem *p3ChatService::locked_makeOwnCustomStateStringItem() { - RsStackMutex stack(mChatMtx); /********** STACK LOCKED MTX ******/ + /* INTERNAL HELPER: No internal mutex to prevent recursion. + * The caller MUST hold mChatMtx before calling this. */ RsChatStatusItem *ci = new RsChatStatusItem(); ci->flags = RS_CHAT_FLAG_CUSTOM_STATE ; @@ -1582,9 +1592,8 @@ RsChatStatusItem *p3ChatService::makeOwnCustomStateStringItem() return ci ; } -RsChatAvatarItem *p3ChatService::makeOwnAvatarItem() +RsChatAvatarItem *p3ChatService::locked_makeOwnAvatarItem() { - /* Mutex is removed here to prevent recursive deadlock from parent calls */ RsChatAvatarItem *ci = new RsChatAvatarItem(); if(_own_avatar != nullptr) @@ -1601,9 +1610,12 @@ void p3ChatService::sendAvatarJpegData(const RsPeerId& peer_id) std::cerr << "p3chatservice: sending requested for peer " << peer_id << ", data=" << (void*)_own_avatar << std::endl ; #endif + /* We need to lock here because locked_makeOwnAvatarItem requires it */ + RS_STACK_MUTEX(mChatMtx); + if(_own_avatar != NULL) { - RsChatAvatarItem *ci = makeOwnAvatarItem(); + RsChatAvatarItem *ci = locked_makeOwnAvatarItem(); ci->PeerId(peer_id); // take avatar, and embed it into a std::string. @@ -1628,7 +1640,9 @@ void p3ChatService::sendCustomState(const RsPeerId& peer_id){ std::cerr << "p3chatservice: sending requested status string for peer " << peer_id << std::endl ; #endif - RsChatStatusItem *cs = makeOwnCustomStateStringItem(); + /* We lock here, then call the 'locked_' helper */ + RS_STACK_MUTEX(mChatMtx); + RsChatStatusItem *cs = locked_makeOwnCustomStateStringItem(); cs->PeerId(peer_id); sendChatItem(cs); @@ -1636,153 +1650,168 @@ std::cerr << "p3chatservice: sending requested status string for peer " << peer_ bool p3ChatService::loadList(std::list& load) { - RsDbg() << "AVATAR entering loadList, total items: " << (int)load.size(); - - for(std::list::iterator it(load.begin()); it != load.end(); ) - { - bool item_handled = false; - RsItem *item = *it; - - /* 1. Attempt to handle Avatar data (local p3ChatService logic) */ - RsConfigKeyValueSet *kvs = dynamic_cast(item); - if(kvs) - { - /* Fixed: Using 'kv' to match the loop variable name */ - for(auto const& kv : kvs->tlvkvs.pairs) - { - if(kv.key == "AV_CACHE:OWN") - { - std::vector decoded_data = Radix64::decode(kv.value); - if(!decoded_data.empty() && decoded_data.size() <= MAX_AVATAR_JPEG_SIZE) - { - RS_STACK_MUTEX(mChatMtx); - if(_own_avatar != NULL) delete _own_avatar; - _own_avatar = new AvatarInfo(decoded_data.data(), (uint32_t)decoded_data.size()); - item_handled = true; - } - } - else if(kv.key.find("AV_CACHE:") == 0) - { - RsPeerId pid(kv.key.substr(9)); - if(!pid.isNull()) - { - std::vector decoded_data = Radix64::decode(kv.value); - if(!decoded_data.empty() && decoded_data.size() <= MAX_AVATAR_JPEG_SIZE) - { - RS_STACK_MUTEX(mChatMtx); - if (_avatars.count(pid)) delete _avatars[pid]; - _avatars[pid] = new AvatarInfo(decoded_data.data(), (uint32_t)decoded_data.size()); - item_handled = true; - } - } - } - } - } + for(std::list::iterator it(load.begin()); it != load.end(); ) + { + bool item_handled = false; + RsItem *item = *it; - /* 2. Handle original Chat Service items (Status & Outgoing) */ - if (!item_handled) - { - RsChatStatusItem *mitem = dynamic_cast(item); - if(mitem) - { - _custom_status_string = mitem->status_string ; - item_handled = true; - } + /* A. Handle Binary Items (Own Avatar ID 0) */ + RsChatAvatarItem *ai = dynamic_cast(item); + if(ai) + { + RS_STACK_MUTEX(mChatMtx); + RsPeerId pid = ai->PeerId(); + if(pid.isNull() || pid == mServiceCtrl->getOwnId()) { + if(_own_avatar == NULL) _own_avatar = new AvatarInfo(ai->image_data, ai->image_size); + item_handled = true; + } + } - PrivateOugoingMapItem* om = dynamic_cast(item); - if(om) - { - RS_STACK_MUTEX(mChatMtx); - for( auto& pair : om->store ) - privateOutgoingMap.insert(outMP::value_type(pair.first, new RsChatMsgItem(pair.second))); - item_handled = true; - } - } + /* B. Handle Key-Value Set (Peer Avatars - name/kvs convention) */ + RsConfigKeyValueSet *kv = dynamic_cast(item); - /* 3. THE RELAY: This is what restores your lobbies. - * If we haven't handled the item, we ask the parents if they recognize it. */ - if (!item_handled) - { - if (DistributedChatService::processLoadListItem(item)) - { - item_handled = true; - } - } + if(kv) + { + /* Check if the KV set contains peer IDs. Since RsConfigKeyValueSet has no name, + * we iterate and check keys. */ + bool found_avatar = false; + RS_STACK_MUTEX(mChatMtx); +#ifdef CHAT_DEBUG + RsDbg() << "AVATAR p3ChatService::loadList: Checking KvSet with " << kv->tlvkvs.pairs.size() << " pairs."; + if(!kv->tlvkvs.pairs.empty()) { + RsDbg() << "AVATAR p3ChatService::loadList: First key is: " << kv->tlvkvs.pairs.front().key; + } +#endif + for(std::list::const_iterator mit = kv->tlvkvs.pairs.begin(); mit != kv->tlvkvs.pairs.end(); ++mit) + { + // If the key is a valid PeerId, we treat it as an avatar entry + // Only attempt conversion if key format is valid (32 hex chars) to avoid noisy errors + if (mit->key.length() == 32 && std::all_of(mit->key.begin(), mit->key.end(), ::isxdigit)) + { + RsPeerId pid(mit->key); + if (!pid.isNull()) { +#ifdef CHAT_DEBUG + RsDbg() << "AVATAR p3ChatService::loadList: Loading avatar for " << pid << ", size=" << mit->value.size() << "."; +#endif + if (_avatars.count(pid)) delete _avatars[pid]; + _avatars[pid] = new AvatarInfo(mit->value); + found_avatar = true; + } + } + } + if(found_avatar) item_handled = true; + } - if (!item_handled) - { - if (DistantChatService::processLoadListItem(item)) - { - item_handled = true; - } - } + /* C. Handle Status & Messages */ + if (!item_handled) + { + RsChatStatusItem *mitem = dynamic_cast(item); + if(mitem) { + RS_STACK_MUTEX(mChatMtx); + _custom_status_string = mitem->status_string; + item_handled = true; + } + PrivateOugoingMapItem* om = dynamic_cast(item); + if(om) { + RS_STACK_MUTEX(mChatMtx); + for( auto& pair : om->store ) + privateOutgoingMap.insert(outMP::value_type(pair.first, new RsChatMsgItem(pair.second))); + item_handled = true; + } + } - /* 4. Final Decision: Only remove items that were successfully processed */ - if(item_handled) - { - delete item; - it = load.erase(it); - } - else - { - /* If unhandled, move to the next item so other services can see it */ - ++it; - } - } + /* D. RELAY to parents */ + if (!item_handled && DistributedChatService::processLoadListItem(item)) item_handled = true; + if (!item_handled && DistantChatService::processLoadListItem(item)) item_handled = true; - return true ; + if(item_handled) { delete item; it = load.erase(it); } + else { ++it; } + } + return true; } bool p3ChatService::saveList(bool& cleanup, std::list& list) { - cleanup = true; + cleanup = true; + RS_STACK_MUTEX(mChatMtx); + RsPeerId ownId = mServiceCtrl->getOwnId(); - /* 1. Save avatars */ - if(_own_avatar != NULL && _own_avatar->_image_size > 0) - { - RsConfigKeyValueSet *item = new RsConfigKeyValueSet(); - RsTlvKeyValue kv; kv.key = "AV_CACHE:OWN"; - Radix64::encode(_own_avatar->_image_data, _own_avatar->_image_size, kv.value); - item->tlvkvs.pairs.push_back(kv); - list.push_back(item); - } + /* 1. Save OWN avatar: Binary item with ID 0 */ + if(_own_avatar != NULL && _own_avatar->_image_size > 0) + { + RsChatAvatarItem *ai = locked_makeOwnAvatarItem(); + ai->PeerId(RsPeerId()); + list.push_back(ai); + } - for(auto const& [pid, info] : _avatars) - { - if (info != NULL && info->_image_size > 0 && !pid.isNull()) - { - RsConfigKeyValueSet *item = new RsConfigKeyValueSet(); - RsTlvKeyValue kv; kv.key = "AV_CACHE:" + pid.toStdString(); - Radix64::encode(info->_image_data, info->_image_size, kv.value); - item->tlvkvs.pairs.push_back(kv); - list.push_back(item); - } - } + /* 2. Save PEER avatars: Key-Value Set (name/kvs convention) */ + if (!_avatars.empty()) + { +#ifdef CHAT_DEBUG + RsDbg() << "AVATAR p3ChatService::saveList: Total avatars in memory map: " << _avatars.size(); +#endif + RsConfigKeyValueSet *kv = NULL; + int count_in_chunk = 0; + const int MAX_AVATARS_PER_CHUNK = 10; - /* 2. Original developer items (Messages & Status) */ - mChatMtx.lock(); + for(std::map::iterator it = _avatars.begin(); it != _avatars.end(); ++it) + { + if (it->second != NULL && it->second->_image_size > 0 && !it->first.isNull() && it->first != ownId) + { + if (kv == NULL) { + kv = new RsConfigKeyValueSet(); + count_in_chunk = 0; + } - PrivateOugoingMapItem* om = new PrivateOugoingMapItem; - for( auto& pair : privateOutgoingMap ) - om->store.insert(std::map::value_type(pair.first, *pair.second)); - list.push_back(om); +#ifdef CHAT_DEBUG + RsDbg() << "AVATAR p3ChatService::saveList: Saving avatar for " << it->first << ", size=" << it->second->_image_size << "."; +#endif + RsTlvKeyValue pair; + pair.key = it->first.toStdString(); + pair.value = it->second->toRadix64(); + kv->tlvkvs.pairs.push_back(pair); + count_in_chunk++; + + if (count_in_chunk >= MAX_AVATARS_PER_CHUNK) { + list.push_back(kv); + kv = NULL; + } + } +#ifdef CHAT_DEBUG + else if (it->first != ownId) { + RsDbg() << "AVATAR p3ChatService::saveList: SKIPPING avatar for " << it->first + << " (Size=" << (it->second ? it->second->_image_size : -1) + << ", ValidPtr=" << (it->second != NULL) + << ", ValidId=" << !it->first.isNull() << ")"; + } +#endif + } + + if(kv != NULL) { + if(!kv->tlvkvs.pairs.empty()) + list.push_back(kv); + else + delete kv; + } + } - RsChatStatusItem *di = new RsChatStatusItem ; - di->status_string = _custom_status_string ; - di->flags = RS_CHAT_FLAG_CUSTOM_STATE ; - list.push_back(di); + /* 3. Status and messages */ + list.push_back(locked_makeOwnCustomStateStringItem()); + PrivateOugoingMapItem* om = new PrivateOugoingMapItem; + for( auto& pair : privateOutgoingMap ) + om->store.insert(std::map::value_type(pair.first, *pair.second)); + list.push_back(om); - /* 3. CRITICAL: Save Chat Lobbies and Distant Chat subscriptions */ - DistributedChatService::addToSaveList(list) ; - DistantChatService::addToSaveList(list) ; + /* 4. Parent Relays */ + DistributedChatService::addToSaveList(list); + DistantChatService::addToSaveList(list); - return true; + return true; } void p3ChatService::saveDone() { - /* unlock mutex */ - mChatMtx.unlock(); /****** MUTEX UNLOCKED *******/ + /* Empty because we now use RsStackMutex in saveList */ } RsSerialiser *p3ChatService::setupSerialiser() diff --git a/src/chat/p3chatservice.h b/src/chat/p3chatservice.h index a196e535c..33d00c000 100644 --- a/src/chat/p3chatservice.h +++ b/src/chat/p3chatservice.h @@ -283,8 +283,8 @@ class p3ChatService : /// if so, the chat item will be turned to NULL bool locked_checkAndRebuildPartialMessage(RsChatMsgItem *&) ; - RsChatAvatarItem *makeOwnAvatarItem() ; - RsChatStatusItem *makeOwnCustomStateStringItem() ; + RsChatAvatarItem *locked_makeOwnAvatarItem() ; + RsChatStatusItem *locked_makeOwnCustomStateStringItem() ; p3ServiceControl *mServiceCtrl; p3LinkMgr *mLinkMgr; From 5bece8166fbe31e67dd38989cd9558c91eba9268 Mon Sep 17 00:00:00 2001 From: jolavillette Date: Tue, 27 Jan 2026 22:13:15 +0100 Subject: [PATCH 6/8] Implement avatar timestamp handshake to reduce bandwidth usage --- src/chat/p3chatservice.cc | 79 +++++++++++++++++++++++++++++++++++---- src/chat/p3chatservice.h | 5 +++ src/chat/rschatitems.cc | 6 +++ src/chat/rschatitems.h | 12 ++++++ 4 files changed, 95 insertions(+), 7 deletions(-) diff --git a/src/chat/p3chatservice.cc b/src/chat/p3chatservice.cc index f3b751331..49161c517 100644 --- a/src/chat/p3chatservice.cc +++ b/src/chat/p3chatservice.cc @@ -43,7 +43,7 @@ #include "chat/p3chatservice.h" #include "rsitems/rsconfigitems.h" -#define CHAT_DEBUG 1 +//#define CHAT_DEBUG 1 RsChats *rsChats = nullptr; @@ -359,7 +359,7 @@ class p3ChatService::AvatarInfo { public: /* Fix: Initialize in the exact order of declaration (_image_size then _image_data) */ - AvatarInfo() : _image_size(0), _image_data(NULL), _peer_is_new(false), _own_is_new(false), _last_request_time(0) {} + AvatarInfo() : _image_size(0), _image_data(NULL), _peer_is_new(false), _own_is_new(false), _last_request_time(0), _timestamp(0) {} ~AvatarInfo() { if (_image_data) free(_image_data); } @@ -377,6 +377,7 @@ class p3ChatService::AvatarInfo _peer_is_new = false; _own_is_new = false; _last_request_time = 0; + _timestamp = 0; } /* Fix: Use the 3-argument encode required by your SDK */ @@ -399,7 +400,14 @@ class p3ChatService::AvatarInfo } /* Order must be identical here too */ - AvatarInfo(const unsigned char *jpeg_data, int size) : _image_size(0), _image_data(NULL) { init(jpeg_data, size); } + AvatarInfo(const unsigned char *jpeg_data, int size) : _image_size(0), _image_data(NULL) + { + init(jpeg_data, size); + _peer_is_new = false; + _own_is_new = false; + _last_request_time = 0; + _timestamp = time(NULL); + } void toUnsignedChar(unsigned char *& data, uint32_t& size) const { @@ -414,6 +422,7 @@ class p3ChatService::AvatarInfo bool _peer_is_new; bool _own_is_new; time_t _last_request_time; + time_t _timestamp; }; void p3ChatService::sendGroupChatStatusString(const std::string& status_string) @@ -900,6 +909,9 @@ void p3ChatService::handleIncomingItem(RsItem *item) case RS_PKT_SUBTYPE_CHAT_AVATAR: handleRecvChatAvatarItem(dynamic_cast(item)); break; + case RS_PKT_SUBTYPE_CHAT_AVATAR_INFO: + handleRecvChatAvatarInfoItem(dynamic_cast(item)); + break; default: { static int already = false; @@ -1403,7 +1415,7 @@ void p3ChatService::setOwnNodeAvatarData(const unsigned char *data, int size) for(std::set::iterator it = onlineList.begin(); it != onlineList.end(); ++it) { RsDbg() << "AVATAR broadcasting to peer: " << it->toStdString().c_str(); - sendAvatarJpegData(*it); + sendAvatarInfo(*it); } IndicateConfigChanged(); @@ -1648,6 +1660,61 @@ std::cerr << "p3chatservice: sending requested status string for peer " << peer_ sendChatItem(cs); } +RsChatAvatarInfoItem *p3ChatService::locked_makeOwnAvatarInfoItem() +{ + RsChatAvatarInfoItem *ci = new RsChatAvatarInfoItem(); + if(_own_avatar != nullptr) + { + ci->timestamp = (uint32_t)_own_avatar->_timestamp; + } + return ci; +} + +void p3ChatService::sendAvatarInfo(const RsPeerId& peer_id) +{ +#ifdef CHAT_DEBUG + RsDbg() << "AVATAR p3ChatService::sendAvatarInfo: Sending Info to " << peer_id; +#endif + RS_STACK_MUTEX(mChatMtx); + if(_own_avatar != nullptr) + { + RsChatAvatarInfoItem *ci = locked_makeOwnAvatarInfoItem(); + ci->PeerId(peer_id); + sendChatItem(ci); + } +} + +void p3ChatService::handleRecvChatAvatarInfoItem(RsChatAvatarInfoItem *item) +{ + RsPeerId pid = item->PeerId(); + if(pid.isNull()) return; + + RS_STACK_MUTEX(mChatMtx); + std::map::iterator it = _avatars.find(pid); + + bool need_update = false; + if(it == _avatars.end()) + { + need_update = true; + } + else + { + if(it->second->_timestamp < item->timestamp) + { + need_update = true; + } + } + + if(need_update) + { +#ifdef CHAT_DEBUG + RsDbg() << "AVATAR p3ChatService::handleRecvChatAvatarInfoItem: Peer " << pid << " has newer avatar (remote TS=" << item->timestamp << "). Requesting."; +#endif + sendAvatarRequest(pid); + } +} + + bool p3ChatService::loadList(std::list& load) { for(std::list::iterator it(load.begin()); it != load.end(); ) @@ -1871,11 +1938,9 @@ void p3ChatService::statusChange(const std::list &plist) IndicateConfigChanged(); /* AVATAR Handshake on connection */ - RsDbg() << "AVATAR peer connected, initiating sync with: " << it->id.toStdString().c_str(); - sendAvatarRequest(it->id); if(_own_avatar != nullptr && _own_avatar->_image_size > 0) { - sendAvatarJpegData(it->id); + sendAvatarInfo(it->id); } } else if (it->actions & RS_SERVICE_PEER_REMOVED) diff --git a/src/chat/p3chatservice.h b/src/chat/p3chatservice.h index 33d00c000..ac0be8f50 100644 --- a/src/chat/p3chatservice.h +++ b/src/chat/p3chatservice.h @@ -256,6 +256,9 @@ class p3ChatService : /// Send avatar info to peer in jpeg format. void sendAvatarJpegData(const RsPeerId& peer_id) ; + /// Send avatar info (timestamp) to peer. + void sendAvatarInfo(const RsPeerId& peer_id); + /// Send custom state info to peer void sendCustomState(const RsPeerId& peer_id); @@ -268,6 +271,7 @@ class p3ChatService : void handleRecvChatStatusItem(RsChatStatusItem *item) ; void handleRecvChatAvatarItem(RsChatAvatarItem *item) ; + void handleRecvChatAvatarInfoItem(RsChatAvatarInfoItem *item); /// Sends a request for an avatar to the peer of given id void sendAvatarRequest(const RsPeerId& peer_id) ; @@ -284,6 +288,7 @@ class p3ChatService : bool locked_checkAndRebuildPartialMessage(RsChatMsgItem *&) ; RsChatAvatarItem *locked_makeOwnAvatarItem() ; + RsChatAvatarInfoItem *locked_makeOwnAvatarInfoItem() ; RsChatStatusItem *locked_makeOwnCustomStateStringItem() ; p3ServiceControl *mServiceCtrl; diff --git a/src/chat/rschatitems.cc b/src/chat/rschatitems.cc index 6d365449e..a9bfbf1eb 100644 --- a/src/chat/rschatitems.cc +++ b/src/chat/rschatitems.cc @@ -53,6 +53,7 @@ RsItem *RsChatSerialiser::create_item(uint16_t service_id,uint8_t item_sub_id) c case RS_PKT_SUBTYPE_CHAT_LOBBY_LIST: return new RsChatLobbyListItem(); case RS_PKT_SUBTYPE_CHAT_LOBBY_CONFIG: return new RsChatLobbyConfigItem(); case RS_PKT_SUBTYPE_SUBSCRIBED_CHAT_LOBBY_CONFIG: return new RsSubscribedChatLobbyConfigItem(); + case RS_PKT_SUBTYPE_CHAT_AVATAR_INFO: return new RsChatAvatarInfoItem(); case RS_PKT_SUBTYPE_OUTGOING_MAP: return new PrivateOugoingMapItem(); default: std::cerr << "Unknown packet type in chat!" << std::endl; @@ -164,6 +165,11 @@ void RsChatAvatarItem::serial_process(RsGenericSerializer::SerializeJob j,RsGene RsTypeSerializer::serial_process(j,ctx,b,"image data") ; } +void RsChatAvatarInfoItem::serial_process(RsGenericSerializer::SerializeJob j,RsGenericSerializer::SerializeContext& ctx) +{ + RsTypeSerializer::serial_process(j,ctx,timestamp,"timestamp"); +} + void RsSubscribedChatLobbyConfigItem::serial_process(RsGenericSerializer::SerializeJob j,RsGenericSerializer::SerializeContext& ctx) { info.serial_process(j,ctx); diff --git a/src/chat/rschatitems.h b/src/chat/rschatitems.h index f491dc63f..07911d7ae 100644 --- a/src/chat/rschatitems.h +++ b/src/chat/rschatitems.h @@ -84,6 +84,7 @@ const uint8_t RS_PKT_SUBTYPE_CHAT_LOBBY_INVITE = 0x1B ; const uint8_t RS_PKT_SUBTYPE_OUTGOING_MAP = 0x1C ; const uint8_t RS_PKT_SUBTYPE_SUBSCRIBED_CHAT_LOBBY_CONFIG = 0x1D ; +const uint8_t RS_PKT_SUBTYPE_CHAT_AVATAR_INFO = 0x1E ; typedef uint64_t ChatLobbyId ; typedef uint64_t ChatLobbyMsgId ; @@ -362,6 +363,17 @@ class RsChatAvatarItem: public RsChatItem unsigned char* image_data ; /// image data }; +class RsChatAvatarInfoItem: public RsChatItem +{ +public: + RsChatAvatarInfoItem(): RsChatItem(RS_PKT_SUBTYPE_CHAT_AVATAR_INFO), timestamp(0) {} + virtual ~RsChatAvatarInfoItem() {} + + void serial_process(RsGenericSerializer::SerializeJob j,RsGenericSerializer::SerializeContext& ctx) override; + + uint32_t timestamp; +}; + struct PrivateOugoingMapItem : RsChatItem { From 1676330d4b7e2b84dfbdab0a12cf8099aad6947c Mon Sep 17 00:00:00 2001 From: jolavillette Date: Wed, 28 Jan 2026 15:13:42 +0100 Subject: [PATCH 7/8] fix persistence by updating TS when an avatar is received, and request missing avatar on peer connection for backward compatibility --- src/chat/p3chatservice.cc | 98 ++++++++++++++++++++++++++++++++------- 1 file changed, 81 insertions(+), 17 deletions(-) diff --git a/src/chat/p3chatservice.cc b/src/chat/p3chatservice.cc index 49161c517..d676a09c1 100644 --- a/src/chat/p3chatservice.cc +++ b/src/chat/p3chatservice.cc @@ -23,6 +23,8 @@ #include #include #include +#include +#include #include "util/rsdir.h" #include "util/radix64.h" @@ -363,39 +365,66 @@ class p3ChatService::AvatarInfo ~AvatarInfo() { if (_image_data) free(_image_data); } - /* Fix: Constructor for Radix64 loading (Peer Storage) */ - AvatarInfo(const std::string& r64_data) : _image_size(0), _image_data(NULL) + /* Fix: Constructor for combined TS + Radix64 loading */ + AvatarInfo(const std::string& encoded_data) : _image_size(0), _image_data(NULL) { - if (!r64_data.empty()) + if (encoded_data.length() > 16) { - /* Use the vector-based decode required by your SDK */ + std::string ts_hex = encoded_data.substr(0, 16); + std::string r64_data = encoded_data.substr(16); + + try { + _timestamp = std::stoull(ts_hex, nullptr, 16); + } catch (...) { + _timestamp = 0; + } + std::vector tmp = Radix64::decode(r64_data); if (!tmp.empty()) { init(tmp.data(), (int)tmp.size()); } } + else if (!encoded_data.empty()) + { + /* Backward compatibility: if no TS prefix, just decode as Radix64 */ + std::vector tmp = Radix64::decode(encoded_data); + if (!tmp.empty()) { + init(tmp.data(), (int)tmp.size()); + } + /* Assign current time as timestamp for old avatars to prevent re-download */ + _timestamp = time(NULL); + } + else + { + _timestamp = 0; + } _peer_is_new = false; _own_is_new = false; _last_request_time = 0; - _timestamp = 0; } - /* Fix: Use the 3-argument encode required by your SDK */ + /* Fix: Returns TS (16 hex chars) + Radix64 image data */ std::string toRadix64() const { + std::stringstream ss; + ss << std::setfill('0') << std::setw(16) << std::hex << (uint64_t)_timestamp; + std::string out; if (_image_data && _image_size > 0) { Radix64::encode(_image_data, (size_t)_image_size, out); } - return out; + ss << out; + return ss.str(); } void init(const unsigned char *jpeg_data, int size) { + if (_image_data) { free(_image_data); _image_data = NULL; _image_size = 0; } if (size > 0) { _image_size = size; _image_data = (unsigned char*)rs_malloc(size); memcpy(_image_data, jpeg_data, size); + _timestamp = time(NULL); // Update timestamp when image is set } } @@ -1454,8 +1483,15 @@ void p3ChatService::receiveAvatarJpegData(RsChatAvatarItem *ci) RS_STACK_MUTEX(mChatMtx); RsDbg() << "AVATAR: [RECV] Received valid avatar for peer: " << pid.toStdString(); - if (_avatars.count(pid)) delete _avatars[pid]; - _avatars[pid] = new AvatarInfo(ci->image_data, ci->image_size); + + if (_avatars.count(pid)) + { + _avatars[pid]->init(ci->image_data, ci->image_size); + } + else + { + _avatars[pid] = new AvatarInfo(ci->image_data, ci->image_size); + } _avatars[pid]->_peer_is_new = true; IndicateConfigChanged(); @@ -1695,12 +1731,15 @@ void p3ChatService::handleRecvChatAvatarInfoItem(RsChatAvatarInfoItem *item) bool need_update = false; if(it == _avatars.end()) { + _avatars[pid] = new AvatarInfo(); + _avatars[pid]->_timestamp = (time_t)item->timestamp; need_update = true; } else { - if(it->second->_timestamp < item->timestamp) + if((uint32_t)it->second->_timestamp < item->timestamp) { + it->second->_timestamp = (time_t)item->timestamp; need_update = true; } } @@ -1757,14 +1796,22 @@ bool p3ChatService::loadList(std::list& load) { RsPeerId pid(mit->key); if (!pid.isNull()) { -#ifdef CHAT_DEBUG - RsDbg() << "AVATAR p3ChatService::loadList: Loading avatar for " << pid << ", size=" << mit->value.size() << "."; -#endif + RsDbg() << "AVATAR p3ChatService::loadList: Loading avatar for " << pid << ", encoded_size=" << mit->value.size() << "."; if (_avatars.count(pid)) delete _avatars[pid]; - _avatars[pid] = new AvatarInfo(mit->value); + _avatars[pid] = new AvatarInfo(mit->value); + RsDbg() << "AVATAR p3ChatService::loadList: Loaded avatar for " << pid << ", image_size=" << _avatars[pid]->_image_size << ", timestamp=" << _avatars[pid]->_timestamp << "."; found_avatar = true; } } + else if (mit->key == "OWN_AVATAR_TS") + { + if (_own_avatar) { + try { + _own_avatar->_timestamp = (time_t)std::stoull(mit->value, nullptr, 10); + } catch(...) {} + } + found_avatar = true; + } } if(found_avatar) item_handled = true; } @@ -1809,6 +1856,14 @@ bool p3ChatService::saveList(bool& cleanup, std::list& list) RsChatAvatarItem *ai = locked_makeOwnAvatarItem(); ai->PeerId(RsPeerId()); list.push_back(ai); + + /* Save own TS in a KV set */ + RsConfigKeyValueSet *okv = new RsConfigKeyValueSet(); + RsTlvKeyValue pair; + pair.key = "OWN_AVATAR_TS"; + pair.value = std::to_string((uint64_t)_own_avatar->_timestamp); + okv->tlvkvs.pairs.push_back(pair); + list.push_back(okv); } /* 2. Save PEER avatars: Key-Value Set (name/kvs convention) */ @@ -1830,12 +1885,11 @@ bool p3ChatService::saveList(bool& cleanup, std::list& list) count_in_chunk = 0; } -#ifdef CHAT_DEBUG - RsDbg() << "AVATAR p3ChatService::saveList: Saving avatar for " << it->first << ", size=" << it->second->_image_size << "."; -#endif + RsDbg() << "AVATAR p3ChatService::saveList: Saving avatar for " << it->first << ", image_size=" << it->second->_image_size << ", timestamp=" << it->second->_timestamp << "."; RsTlvKeyValue pair; pair.key = it->first.toStdString(); pair.value = it->second->toRadix64(); + RsDbg() << "AVATAR p3ChatService::saveList: Encoded value size=" << pair.value.size() << ", first 32 chars: " << pair.value.substr(0, std::min((size_t)32, pair.value.size())); kv->tlvkvs.pairs.push_back(pair); count_in_chunk++; @@ -1942,6 +1996,16 @@ void p3ChatService::statusChange(const std::list &plist) { sendAvatarInfo(it->id); } + + /* Request peer's avatar only if we don't have one (backward compatibility with old code) */ + { + RS_STACK_MUTEX(mChatMtx); + std::map::const_iterator it_avatar = _avatars.find(it->id); + if(it_avatar == _avatars.end() || it_avatar->second->_image_size == 0) + { + sendAvatarRequest(it->id); + } + } } else if (it->actions & RS_SERVICE_PEER_REMOVED) { From 5608ae946f696969af37aa2ad7d5aadea8a2861c Mon Sep 17 00:00:00 2001 From: jolavillette Date: Wed, 28 Jan 2026 15:40:51 +0100 Subject: [PATCH 8/8] remove AVATAR debug messages --- src/chat/p3chatservice.cc | 40 +++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/src/chat/p3chatservice.cc b/src/chat/p3chatservice.cc index d676a09c1..2fc01215c 100644 --- a/src/chat/p3chatservice.cc +++ b/src/chat/p3chatservice.cc @@ -46,6 +46,7 @@ #include "rsitems/rsconfigitems.h" //#define CHAT_DEBUG 1 +//#define AVATAR_DEBUG 1 RsChats *rsChats = nullptr; @@ -1410,7 +1411,9 @@ void p3ChatService::setOwnNodeAvatarData(const unsigned char *data, int size) * preventing the deadlock you saw in GDB. */ RsStackMutex stack(mChatMtx); /********** STACK LOCKED MTX ******/ +#ifdef AVATAR_DEBUG RsDbg() << "AVATAR setting own node avatar data, size: " << size; +#endif #ifdef CHAT_DEBUG std::cerr << "p3chatservice: Setting own avatar to new image." << std::endl ; @@ -1443,7 +1446,9 @@ void p3ChatService::setOwnNodeAvatarData(const unsigned char *data, int size) for(std::set::iterator it = onlineList.begin(); it != onlineList.end(); ++it) { +#ifdef AVATAR_DEBUG RsDbg() << "AVATAR broadcasting to peer: " << it->toStdString().c_str(); +#endif sendAvatarInfo(*it); } @@ -1477,13 +1482,16 @@ void p3ChatService::receiveAvatarJpegData(RsChatAvatarItem *ci) /* Safety: Do not let network packets overwrite local 'Self' data */ if(pid.isNull() || (!ownId.isNull() && pid == ownId)) { +#ifdef AVATAR_DEBUG RsDbg() << "AVATAR: [RECV] Ignored incoming avatar packet identifying as SELF."; +#endif return; } RS_STACK_MUTEX(mChatMtx); +#ifdef AVATAR_DEBUG RsDbg() << "AVATAR: [RECV] Received valid avatar for peer: " << pid.toStdString(); - +#endif if (_avatars.count(pid)) { _avatars[pid]->init(ci->image_data, ci->image_size); @@ -1578,7 +1586,7 @@ void p3ChatService::getAvatarData(const RsPeerId& peer_id,unsigned char *& data, time_t now = time(NULL); if (now - it->second->_last_request_time > 60) { -#ifdef CHAT_DEBUG +#ifdef AVATAR_DEBUG RsDbg() << "AVATAR p3ChatService::getAvatarData: No avatar for peer " << peer_id << ". Requesting it (throttled)."; #endif it->second->_last_request_time = now; @@ -1602,7 +1610,7 @@ void p3ChatService::sendAvatarRequest(const RsPeerId& peer_id) ci->sendTime = time(NULL); ci->message.erase(); -#ifdef CHAT_DEBUG +#ifdef AVATAR_DEBUG RsDbg() << "AVATAR p3ChatService::sendAvatarRequest: sending request for avatar to peer " << peer_id; #endif @@ -1708,7 +1716,7 @@ RsChatAvatarInfoItem *p3ChatService::locked_makeOwnAvatarInfoItem() void p3ChatService::sendAvatarInfo(const RsPeerId& peer_id) { -#ifdef CHAT_DEBUG +#ifdef AVATAR_DEBUG RsDbg() << "AVATAR p3ChatService::sendAvatarInfo: Sending Info to " << peer_id; #endif RS_STACK_MUTEX(mChatMtx); @@ -1746,7 +1754,7 @@ void p3ChatService::handleRecvChatAvatarInfoItem(RsChatAvatarInfoItem *item) if(need_update) { -#ifdef CHAT_DEBUG +#ifdef AVATAR_DEBUG RsDbg() << "AVATAR p3ChatService::handleRecvChatAvatarInfoItem: Peer " << pid << " has newer avatar (remote TS=" << item->timestamp << "). Requesting."; #endif sendAvatarRequest(pid); @@ -1782,7 +1790,7 @@ bool p3ChatService::loadList(std::list& load) * we iterate and check keys. */ bool found_avatar = false; RS_STACK_MUTEX(mChatMtx); -#ifdef CHAT_DEBUG +#ifdef AVATAR_DEBUG RsDbg() << "AVATAR p3ChatService::loadList: Checking KvSet with " << kv->tlvkvs.pairs.size() << " pairs."; if(!kv->tlvkvs.pairs.empty()) { RsDbg() << "AVATAR p3ChatService::loadList: First key is: " << kv->tlvkvs.pairs.front().key; @@ -1796,10 +1804,14 @@ bool p3ChatService::loadList(std::list& load) { RsPeerId pid(mit->key); if (!pid.isNull()) { - RsDbg() << "AVATAR p3ChatService::loadList: Loading avatar for " << pid << ", encoded_size=" << mit->value.size() << "."; - if (_avatars.count(pid)) delete _avatars[pid]; - _avatars[pid] = new AvatarInfo(mit->value); - RsDbg() << "AVATAR p3ChatService::loadList: Loaded avatar for " << pid << ", image_size=" << _avatars[pid]->_image_size << ", timestamp=" << _avatars[pid]->_timestamp << "."; +#ifdef AVATAR_DEBUG + RsDbg() << "AVATAR p3ChatService::loadList: Loading avatar for " << pid << ", encoded_size=" << mit->value.size() << "."; +#endif + if (_avatars.count(pid)) delete _avatars[pid]; + _avatars[pid] = new AvatarInfo(mit->value); +#ifdef AVATAR_DEBUG + RsDbg() << "AVATAR p3ChatService::loadList: Loaded avatar for " << pid << ", image_size=" << _avatars[pid]->_image_size << ", timestamp=" << _avatars[pid]->_timestamp << "."; +#endif found_avatar = true; } } @@ -1869,7 +1881,7 @@ bool p3ChatService::saveList(bool& cleanup, std::list& list) /* 2. Save PEER avatars: Key-Value Set (name/kvs convention) */ if (!_avatars.empty()) { -#ifdef CHAT_DEBUG +#ifdef AVATAR_DEBUG RsDbg() << "AVATAR p3ChatService::saveList: Total avatars in memory map: " << _avatars.size(); #endif RsConfigKeyValueSet *kv = NULL; @@ -1885,11 +1897,15 @@ bool p3ChatService::saveList(bool& cleanup, std::list& list) count_in_chunk = 0; } +#ifdef AVATAR_DEBUG RsDbg() << "AVATAR p3ChatService::saveList: Saving avatar for " << it->first << ", image_size=" << it->second->_image_size << ", timestamp=" << it->second->_timestamp << "."; +#endif RsTlvKeyValue pair; pair.key = it->first.toStdString(); pair.value = it->second->toRadix64(); +#ifdef AVATAR_DEBUG RsDbg() << "AVATAR p3ChatService::saveList: Encoded value size=" << pair.value.size() << ", first 32 chars: " << pair.value.substr(0, std::min((size_t)32, pair.value.size())); +#endif kv->tlvkvs.pairs.push_back(pair); count_in_chunk++; @@ -1898,7 +1914,7 @@ bool p3ChatService::saveList(bool& cleanup, std::list& list) kv = NULL; } } -#ifdef CHAT_DEBUG +#ifdef AVATAR_DEBUG else if (it->first != ownId) { RsDbg() << "AVATAR p3ChatService::saveList: SKIPPING avatar for " << it->first << " (Size=" << (it->second ? it->second->_image_size : -1)