[WIP] Avatar persistence and synchronization by jolavillette · Pull Request #248 · RetroShare/libretroshare

jolavillette · 2026-01-22T17:27:24Z

Code and comment by Gemini

This summary describes the technical implementation of the new avatar persistence and synchronization system for RetroShare's Chat Service.

Core Implementation Overview
The goal of these changes was to provide a reliable, persistent, and proactive avatar exchange mechanism that operates silently in the background without interrupting the user experience or breaking existing services like Chat Lobbies.

Persistence and Data Integrity
To ensure avatars survive application restarts, the storage logic was migrated to a Key-Value set system within the configuration.

saveList: Now encodes the local node's avatar and all cached peer avatars into RsConfigKeyValueSet items using Radix64 encoding. It explicitly calls parent services to ensure Chat Lobby subscriptions are also saved.

loadList: Implements a non-destructive "Pass-through" logic. It extracts avatar data from KV sets but relays any unrecognized items to DistributedChatService::processLoadListItem and DistantChatService::processLoadListItem.

Lobby Protection: This delegation ensures that shared configuration containers containing lobby data or GXS identities are not deleted by the Chat Service during the loading phase.

Real-time Synchronization & Handshaking
The system now proactively ensures that avatars are up-to-date across the network through two main triggers:

Broadcast on Update (setOwnNodeAvatarData): When the user changes their avatar, the service immediately broadcasts the new image data to all currently online peers.

Handshake on Connection (statusChange): As soon as a peer connects, a bidirectional handshake is initiated. The service sends an avatarRequest to the peer and simultaneously pushes the local avatar data to them.

Silent Protocol: These exchanges use direct RsChatAvatarItem packets, which are processed by the remote UI in the background, preventing unnecessary chat window popups.

Thread Safety and Stability
A critical deadlock identified through GDB debugging was resolved to ensure system stability.

Deadlock Prevention: The RsStackMutex was removed from makeOwnAvatarItem to prevent recursive locking when called from parent functions that already hold the mChatMtx lock.

Scoped Locking: In setOwnNodeAvatarData, the mutex is now confined to a specific scope to ensure it is released before entering the network broadcast loop, allowing child functions to acquire the lock safely.

csoler · 2026-01-24T10:40:01Z

src/chat/p3chatservice.cc


-	mChatMtx.lock(); /****** MUTEX LOCKED *******/
+	/* 2. Original developer items (Messages & Status) */
+	mChatMtx.lock(); 


I don't like these .lock() (I know it was here before). It's probably a good opportunity to replace it with a stack mutex.

csoler · 2026-01-24T10:42:44Z

src/chat/p3chatservice.cc

-		ci->PeerId(mServiceCtrl->getOwnId());
+		RsConfigKeyValueSet *item = new RsConfigKeyValueSet();
+		RsTlvKeyValue kv; kv.key = "AV_CACHE:OWN";
+		Radix64::encode(_own_avatar->_image_data, _own_avatar->_image_size, kv.value);


Note: RsConfigKeyValueSet is rather limiting in the type of what would be used (basically strings), which explains the Radix64 format. You would get a smaller print using an explicit encoding in the RsItem, but this will cause problems of backward compatibility.

csoler · 2026-01-24T11:42:41Z

src/chat/p3chatservice.cc

 RsChatAvatarItem *p3ChatService::makeOwnAvatarItem()
 {
-	RsStackMutex stack(mChatMtx); /********** STACK LOCKED MTX ******/
+	/* Mutex is removed here to prevent recursive deadlock from parent calls */


Good practice: if you remove the mutex, rename the method as locked_makeOwnAvatarItem() : this indicates that the method should be called within a mutex-protected region.

csoler · 2026-01-24T11:47:52Z

src/chat/p3chatservice.cc

+	RsDbg() << "AVATAR entering loadList, total items: " << (int)load.size();

-	for(std::list<RsItem*>::const_iterator it(load.begin());it!=load.end();++it)
+	for(std::list<RsItem*>::iterator it(load.begin()); it != load.end(); )


Since you changed the way your own avatar is saved, this is not backward compatible: patched users will lose their avatar and will have to re-define it.

I propose a better alternative: use RsChatAvatarItem as before, setting the PeerId() of the item to the node ID of that avatar. This is backward compatible with what's done now, and also will require less storage since it's not converting to Radix64.

csoler · 2026-01-24T11:48:34Z

src/chat/p3chatservice.cc

+			RsChatStatusItem *mitem = dynamic_cast<RsChatStatusItem *>(item);
+			if(mitem)
+			{
+				_custom_status_string = mitem->status_string ;


why remove the mutex?

csoler · 2026-01-24T11:52:46Z

Use RsChatAvatarItem instead of KeyValueSet => it will be backward compatible and the code is also simpler.

jolavillette · 2026-01-24T17:03:26Z

Following Cyril comments:

Own avatar is saved as a RsChatAvatarItem (existing logic confirmed/preserved), insuring backward compatibility. Saved ID is 0, and is interpreted as our ID, see below.
For peers the legacy RsChatAvatarItem could not be used because its serialization ignores the Peer ID (saving only image data), causing lost IDs (zero) on reload. Implemented custom saving using RsConfigKeyValueSet to persist both Peer ID and Avatar Data. Avatars are saved in chunks (10 items per key-value set) to respect serializer size limits and ensure reliable writing to disk.
Various code improvements

csoler · 2026-01-27T17:21:12Z

src/chat/p3chatservice.cc

-   }
-   _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 ;


How do you keep track of peers to which we haven't sent our own avatar yet?

csoler · 2026-01-27T17:22:14Z

src/chat/p3chatservice.cc

 	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 ;


I assume you left CHAT_DEBUG on. Make sure all debug output is disabled/cleaned before we merge.

csoler · 2026-01-27T17:34:57Z

src/chat/p3chatservice.cc

+            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;


I don't understand the need for this item_handled mechanism. In the end, all items that have been loaded need to be destroyed (unless one was actually stored. Does this happen? We didn't have this check before).

ah I see. Each step would delete the item separately. So it's fine as it is now.

csoler · 2026-01-27T17:41:27Z

src/chat/p3chatservice.cc

+        }
+
+        if(kv != NULL) {
+            if(!kv->tlvkvs.pairs.empty())


this check is not necessary (following the code above it's not possible to have an item with an empty list). It's of course not a problem to keep it.

csoler · 2026-01-27T17:46:30Z

src/chat/p3chatservice.cc

+
+			/* AVATAR Handshake on connection */
+			RsDbg() << "AVATAR peer connected, initiating sync with: " << it->id.toStdString().c_str();
+			sendAvatarRequest(it->id);


This is going to cause sending the avatar at every connection. To make it reliable we should send the TS of the avatar we have, and if the peer has a more recent avatar then it should send it. Looking at the code in sendAvatarRequest() it should only be used if we do nto have the avatar.

csoler · 2026-01-27T17:46:49Z

src/chat/p3chatservice.cc

+			sendAvatarRequest(it->id);
+			if(_own_avatar != nullptr && _own_avatar->_image_size > 0)
+			{
+				sendAvatarJpegData(it->id);


same here. Why would we send our own avatar without a request?

jolavillette · 2026-01-27T21:17:41Z

New packet type: Added RsChatAvatarInfoItem containing only the avatar timestamp (replaces systematic full image broadcasting)
Connection handshake: On peer connection, we now send [sendAvatarInfo()] (timestamp only) instead of [sendAvatarJpegData()] (full image)
Smart pull mechanism:
- Peer receives timestamp and compares with local cache
- If remote timestamp is newer → peer sends [sendAvatarRequest()]
- If timestamp is same/older → no action (bandwidth saved)
Avatar updates: When user changes their avatar, the new timestamp is broadcast to all connected peers, triggering the same pull mechanism
Debug cleanup: Disabled CHAT_DEBUG define (debug blocks kept for future debugging)

…dlock

jolavillette · 2026-01-28T09:57:41Z

Save/restore of avatars is broken since last commit (TS). Stand by for a fix.

…t missing avatar on peer connection for backward compatibility

jolavillette · 2026-01-28T14:16:23Z

Fix Avatar Persistence Regression

Fixes avatar persistence issue introduced by timestamp sync implementation.

Problem: Avatars were not persisting across sessions because _timestamp was not updated when receiving avatar data.

Solution:

Update timestamp in AvatarInfo::init() when image data is set
Request peer avatars on connection if not cached (backward compatibility)

Result: Avatars now persist correctly and work with both old and new nodes.

jolavillette force-pushed the AvatarStorageAndUpdateImprove branch from 93fe7c5 to f7273a3 Compare January 24, 2026 07:58

csoler reviewed Jan 24, 2026

View reviewed changes

jolavillette force-pushed the AvatarStorageAndUpdateImprove branch 2 times, most recently from ba0143c to 9a9813e Compare January 27, 2026 06:29

csoler reviewed Jan 27, 2026

View reviewed changes

jolavillette added 6 commits January 27, 2026 22:21

implement persistence for remote peer avatars using KV sets

7b2cea2

initiate proactive avatar exchange on peer connection

f4fb178

implement direct avatar broadcast on update and on connection

da86348

restore lobby persistence via parent delegation and fix recursive dea…

19af04e

…dlock

code and logic improvement following cyril comment

2e73cfd

Implement avatar timestamp handshake to reduce bandwidth usage

5bece81

jolavillette force-pushed the AvatarStorageAndUpdateImprove branch from 0242ad7 to 5bece81 Compare January 27, 2026 21:21

fix persistence by updating TS when an avatar is received, and reques…

1676330

…t missing avatar on peer connection for backward compatibility

remove AVATAR debug messages

5608ae9

defnax mentioned this pull request Jan 28, 2026

[Qt] Friends Avatars needs to be cached/stored RetroShare/RetroShare#1706

Open

Conversation

jolavillette commented Jan 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

csoler commented Jan 24, 2026

Uh oh!

jolavillette commented Jan 24, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jolavillette commented Jan 27, 2026

Uh oh!

jolavillette commented Jan 28, 2026

Uh oh!

jolavillette commented Jan 28, 2026

Fix Avatar Persistence Regression

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants