From 24b6ef5d1be4b3b447ab4b69a94f6500df7d207f Mon Sep 17 00:00:00 2001 From: jolavillette Date: Tue, 18 Aug 2026 09:27:05 +0200 Subject: [PATCH 1/2] Distant chat: closing a conversation left its contact behind, and always said it worked closeDistantChatConnexion() closes the GXS tunnel and stops there, on a comment wondering whether the contact should go too ("also remove contact. Or do we wait for the notification?"). It should: the entry in mDistantChatContacts is the core's record of an open conversation -- handleOutgoingItem() accepts outgoing items for as long as it is there -- and markDistantChatAsClosed(), the remote-close path, removes it. Nothing removed it when we closed the conversation ourselves, and the web UI showed it: a conversation left with "Leave Chat" kept coming back. The return value was the constant true, so every caller -- the chat window, the web UI, any JSON API client -- was told the conversation had been closed even when there was nothing to close: closeExistingTunnel() answers false and logs "Cannot close distant tunnel connection. No connection openned for tunnel id" once p3GxsTunnelService has pruned a remotely-closed tunnel, which it does on its own ~20s after the close arrives. And the chat-level item that would have removed the contact on the remote-close path lost its sender in 2015 (c521e4ed1), so a still-registered contact with no tunnel behind it is the normal state of a conversation whose peer has left. The function now answers whether anything was actually released -- tunnel, contact, or both -- and logs an error in the one inconsistent case, a closed tunnel with no registered contact. Co-Authored-By: Claude Opus 5 (1M context) Co-Authored-By: Claude Fable 5 --- src/chat/distantchat.cc | 39 ++++++++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/src/chat/distantchat.cc b/src/chat/distantchat.cc index 27b2dd40e..d522b68c7 100644 --- a/src/chat/distantchat.cc +++ b/src/chat/distantchat.cc @@ -327,11 +327,40 @@ bool DistantChatService::getDistantChatStatus(const DistantChatPeerId& tunnel_id bool DistantChatService::closeDistantChatConnexion(const DistantChatPeerId &tunnel_id) { - mGxsTunnels->closeExistingTunnel(RsGxsTunnelId(tunnel_id), DISTANT_CHAT_GXS_TUNNEL_SERVICE_ID) ; - - // also remove contact. Or do we wait for the notification? - - return true ; + // The contact has to go with the tunnel: the entry in mDistantChatContacts + // is the core's record of an open conversation -- handleOutgoingItem() + // accepts outgoing items for as long as it is there -- and + // markDistantChatAsClosed(), the remote-close path, removes it. Nothing + // removed it when we closed the conversation ourselves. + + bool tunnel_closed = mGxsTunnels->closeExistingTunnel( + RsGxsTunnelId(tunnel_id), DISTANT_CHAT_GXS_TUNNEL_SERVICE_ID ); + + bool contact_removed = false; + { + RS_STACK_MUTEX(mDistantChatMtx) ; + + auto it = mDistantChatContacts.find(tunnel_id) ; + + if(it != mDistantChatContacts.end()) + { + mDistantChatContacts.erase(it) ; + contact_removed = true ; + } + } + + // The two can disagree in one direction only: p3GxsTunnelService prunes + // remotely-closed tunnels on its own after 20s, so the tunnel may already + // be gone while the contact is still registered. The converse -- a closed + // tunnel with no contact -- is an anomaly. + if(tunnel_closed && !contact_removed) + std::cerr << "(EE) closeDistantChatConnexion(): tunnel " << tunnel_id << " was closed, but no distant chat contact was registered for it." << std::endl; + + // Answering true whatever happened made every client -- the chat window, + // the web UI, any JSON API caller -- report a conversation as closed when + // nothing had been closed at all, the tunnel having died on its own before. + + return tunnel_closed || contact_removed ; } uint32_t DistantChatService::getDistantChatPermissionFlags() From 21f01cae186be52505a77734f51705511b615d59 Mon Sep 17 00:00:00 2001 From: jolavillette Date: Thu, 3 Sep 2026 06:35:35 +0200 Subject: [PATCH 2/2] Distant chat: tell every client when a conversation is closed locally closeDistantChatConnexion() posted no event. The client that called it knew; the others did not: a conversation opened and then left from the web UI stayed open -- green LED and all -- in the desktop window the core had popped for it. Polling cannot fix that: once the tunnel is gone, getDistantChatStatus() answers the same false for a conversation the peer closed (where the window should stay open) and for one we closed from another client (where it should go); only an explicit event tells the two apart. This adds RsDistantChatEventCode::TUNNEL_STATUS_LOCALLY_CLOSED (0x05) and posts it from closeDistantChatConnexion() whenever something was actually closed, with the tunnel id, so every client of this core -- desktop GUI, web UI, JSON API users -- drops the conversation at the same time. The remote close path is untouched: markDistantChatAsClosed() stays as it was, and REMOTELY_CLOSED keeps coming from notifyTunnelStatus() alone. GUI counterpart: RetroShare/RetroShare#3297 (closes the popup on this event). Co-Authored-By: Claude Opus 5 (1M context) Co-Authored-By: Claude Fable 5 --- src/chat/distantchat.cc | 20 +++++++++++++++++++- src/retroshare/rschats.h | 1 + 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/chat/distantchat.cc b/src/chat/distantchat.cc index d522b68c7..bf6cb1f6c 100644 --- a/src/chat/distantchat.cc +++ b/src/chat/distantchat.cc @@ -360,7 +360,25 @@ bool DistantChatService::closeDistantChatConnexion(const DistantChatPeerId &tunn // the web UI, any JSON API caller -- report a conversation as closed when // nothing had been closed at all, the tunnel having died on its own before. - return tunnel_closed || contact_removed ; + bool closed = tunnel_closed || contact_removed ; + + // The client that asked for the close knows about it; the others do not, + // and a conversation closed from the web UI stayed open in the desktop + // chat window. Polling cannot fix that: once the tunnel is gone, + // getDistantChatStatus() answers the same false for a conversation the + // peer closed (where the window should stay open) and for one we closed + // from another client (where it should go). Only an explicit event tells + // them apart, so post it here -- and every client of this core drops the + // conversation at the same time. + if(closed && rsEvents) + { + auto ev = std::make_shared(); + ev->mEventCode = RsDistantChatEventCode::TUNNEL_STATUS_LOCALLY_CLOSED; + ev->mId = tunnel_id; + rsEvents->postEvent(ev); + } + + return closed ; } uint32_t DistantChatService::getDistantChatPermissionFlags() diff --git a/src/retroshare/rschats.h b/src/retroshare/rschats.h index f15efc513..b8f8b20a9 100644 --- a/src/retroshare/rschats.h +++ b/src/retroshare/rschats.h @@ -249,6 +249,7 @@ enum class RsDistantChatEventCode: uint8_t TUNNEL_STATUS_TUNNEL_DN = 0x02, TUNNEL_STATUS_REMOTELY_CLOSED = 0x03, TUNNEL_STATUS_CONNECTION_REFUSED = 0x04, + TUNNEL_STATUS_LOCALLY_CLOSED = 0x05, // we closed it ourselves (closeDistantChatConnexion), from any client of this core }; struct RsChatLobbyEvent : RsEvent // This event handles events internal to the distributed chat system