From 24b6ef5d1be4b3b447ab4b69a94f6500df7d207f Mon Sep 17 00:00:00 2001 From: jolavillette Date: Tue, 18 Aug 2026 09:27:05 +0200 Subject: [PATCH] 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()