From 3f207e91f6061b637f704f94074ee487b030625f Mon Sep 17 00:00:00 2001 From: Mariano Abad Date: Wed, 26 Aug 2026 07:26:26 -0300 Subject: [PATCH] fix(linux): a session logout should hand the peer to the login screen (#15905) * fix(linux): a session logout should hand the peer to the login screen Logging out closes every window in the session, the connection manager's included, and its close handler kicks every peer with the reason a person gets when they disconnect one by hand. That reason is the one thing the client never retries on, so the remote session dies on a frozen frame instead of reconnecting to the greeter that is already there. The close carries nothing to tell the two apart: measured on KDE, the CM receives no signal and logind still reports the session active at that instant, and the server is killed within a few hundred ms either way, so neither a state check nor a grace period can decide it. What is distinguishable is the ACTION: disconnecting a peer is not the same event as this window going away. So the window-close path now says so, and the server ends the session without poisoning the retry; the Disconnect button and the app's own close control keep kicking exactly as before. Linux only, since that is where a logout closes the window. Verified on plasma/sddm with a client attached: a logout now reconnects to the greeter with no dialog, while closing the manager window still shows Closed manually by the peer. * fix(linux): close the tunnel too, and keep the web build compiling Three seams the first pass missed. The web bridge is hand written, not generated, so the new call needs its stub there or flutter build web stops compiling - and that job is disabled in CI, so it would have gone green. try_port_forward_loop is a second consumer of the same channel and only knew Close, so a forwarded tunnel outlived the window it was supposed to die with. And the variant had landed inside the DRM section, whose comment says everything below it is drm-gated. --- flutter/lib/desktop/pages/server_page.dart | 17 ++++++++++++++++- flutter/lib/models/server_model.dart | 10 +++++++--- flutter/lib/web/bridge.dart | 4 ++++ src/flutter_ffi.rs | 9 +++++++++ src/ipc.rs | 9 +++++++++ src/server/connection.rs | 19 +++++++++++++++++++ src/ui_cm_interface.rs | 9 +++++++++ 7 files changed, 73 insertions(+), 4 deletions(-) diff --git a/flutter/lib/desktop/pages/server_page.dart b/flutter/lib/desktop/pages/server_page.dart index a814b9f7e..b1ca18b2b 100644 --- a/flutter/lib/desktop/pages/server_page.dart +++ b/flutter/lib/desktop/pages/server_page.dart @@ -22,6 +22,14 @@ import '../../models/file_model.dart'; import '../../models/platform_model.dart'; import '../../models/server_model.dart'; +/// Set only by this window's own close control, and only once the user has confirmed. Any other +/// way the window can go - a session logout closing every window, the window manager, a native +/// title-bar button this app does not draw - leaves it false, which is the honest answer: +/// nothing in that close says who asked for it. It lives at file scope because the control that +/// sets it (`ConnectionManagerState`) and the handler that reads it (`_DesktopServerPageState`) +/// are different widgets. +bool _cmClosedByOperator = false; + class DesktopServerPage extends StatefulWidget { const DesktopServerPage({Key? key}) : super(key: key); @@ -55,7 +63,10 @@ class _DesktopServerPageState extends State @override void onWindowClose() { - Future.wait([gFFI.serverModel.closeAll(), gFFI.close()]).then((_) { + // Other platforms keep the old behaviour exactly: the ambiguity this guards against is a + // Linux session logout, which closes every window in the session. + final byOperator = _cmClosedByOperator || !isLinux; + Future.wait([gFFI.serverModel.closeAll(byOperator: byOperator), gFFI.close()]).then((_) { if (isMacOS) { RdPlatformChannel.instance.terminate(); } else { @@ -327,6 +338,7 @@ class ConnectionManagerState extends State var tabController = gFFI.serverModel.tabController; final connLength = tabController.length; if (connLength <= 1) { + _cmClosedByOperator = true; windowManager.close(); return true; } else { @@ -338,6 +350,9 @@ class ConnectionManagerState extends State res = await closeConfirmDialog(); } if (res) { + // After the dialog, never before it: an external close while it is open must not + // inherit an intent the user had not expressed yet. + _cmClosedByOperator = true; windowManager.close(); } return res; diff --git a/flutter/lib/models/server_model.dart b/flutter/lib/models/server_model.dart index 40c94fcf5..6e78ad17f 100644 --- a/flutter/lib/models/server_model.dart +++ b/flutter/lib/models/server_model.dart @@ -738,9 +738,13 @@ class ServerModel with ChangeNotifier { } } - Future closeAll() async { - await Future.wait( - _clients.map((client) => bind.cmCloseConnection(connId: client.id))); + /// `byOperator` false means the CM's window went away rather than a person asking for the + /// peers to go. The sessions end either way; only the close reason differs, and with it + /// whether the peer is allowed to reconnect. See `ipc::Data::CmWindowClosed`. + Future closeAll({bool byOperator = true}) async { + await Future.wait(_clients.map((client) => byOperator + ? bind.cmCloseConnection(connId: client.id) + : bind.cmCloseConnectionWindow(connId: client.id))); _clients.clear(); tabController.state.value.tabs.clear(); if (isAndroid) androidUpdatekeepScreenOn(); diff --git a/flutter/lib/web/bridge.dart b/flutter/lib/web/bridge.dart index f4a082941..087d300c1 100644 --- a/flutter/lib/web/bridge.dart +++ b/flutter/lib/web/bridge.dart @@ -1373,6 +1373,10 @@ class RustdeskImpl { throw UnimplementedError("cmLoginRes"); } + Future cmCloseConnectionWindow({required int connId, dynamic hint}) { + throw UnimplementedError("cmCloseConnectionWindow"); + } + Future cmCloseConnection({required int connId, dynamic hint}) { throw UnimplementedError("cmCloseConnection"); } diff --git a/src/flutter_ffi.rs b/src/flutter_ffi.rs index 4064162ff..6d093cfab 100644 --- a/src/flutter_ffi.rs +++ b/src/flutter_ffi.rs @@ -2197,6 +2197,15 @@ pub fn cm_close_connection(conn_id: i32) { crate::ui_cm_interface::close(conn_id); } +/// The CM window closed. On Linux that is ambiguous - a logout closes it the same way a person +/// does - so it ends the session without the no-retry reason; elsewhere it is a plain close. +pub fn cm_close_connection_window(conn_id: i32) { + #[cfg(target_os = "linux")] + crate::ui_cm_interface::close_window(conn_id); + #[cfg(all(not(target_os = "linux"), not(target_os = "ios")))] + crate::ui_cm_interface::close(conn_id); +} + pub fn cm_remove_disconnected_connection(conn_id: i32) { #[cfg(not(any(target_os = "ios")))] crate::ui_cm_interface::remove(conn_id); diff --git a/src/ipc.rs b/src/ipc.rs index 9e3faab63..804b89db6 100644 --- a/src/ipc.rs +++ b/src/ipc.rs @@ -501,6 +501,15 @@ pub enum Data { ControlPermissionsRemoteModify(Option), #[cfg(target_os = "windows")] FileTransferEnabledState(Option), + /// CM -> server: the connection manager's WINDOW went away, which is not the same event + /// as the operator disconnecting a peer. Linux only, and deliberately: there a session + /// logout closes every window, and the close arrives at the CM indistinguishable from a + /// person clicking it - measured on KDE, the CM gets no signal and logind still reports the + /// session active. So the ambiguous case ends the session WITHOUT the no-retry reason and + /// the peer is allowed to reconnect (landing on the greeter after a logout), while the + /// explicit Disconnect button keeps sending `Close` and kicking for good. + #[cfg(target_os = "linux")] + CmWindowClosed, // --- DRM/KMS capture (opt-in `drm` feature) over the `_drm` service-scoped channel --- // All of the following are `cfg(all(linux, drm))`, so the drm-off IPC wire is byte-identical // to upstream. Protocol on `_drm`: on connect the root service sends `DrmDisplayList`, the diff --git a/src/server/connection.rs b/src/server/connection.rs index 649f045fc..adcab4c88 100644 --- a/src/server/connection.rs +++ b/src/server/connection.rs @@ -642,6 +642,18 @@ impl Connection { conn.on_close("connection manager", true).await; break; } + // The connection manager's window went away rather than a person + // disconnecting this peer. End the session exactly as above, but do not + // send the manual close reason: it is the one thing that stops the peer + // from retrying, and on a logout the retry is the whole point - it is + // what puts the peer back on the login screen a moment later. + #[cfg(target_os = "linux")] + ipc::Data::CmWindowClosed => { + conn.chat_unanswered = false; // seen + conn.file_transferred = false; //seen + conn.on_close("connection manager window closed", true).await; + break; + } ipc::Data::CmErr(e) => { if e != "expected" { // cm closed before connection @@ -1201,6 +1213,13 @@ impl Connection { ipc::Data::Close => { bail!("Close requested from connection manager"); } + // Same end as above: a tunnel must not outlive the window either. + // Only the reason differs, and a port forward carries none - the + // peer sees the tunnel drop and decides for itself. + #[cfg(target_os = "linux")] + ipc::Data::CmWindowClosed => { + bail!("Connection manager window closed"); + } ipc::Data::CmErr(e) => { log::error!("Connection manager error: {e}"); bail!("{e}"); diff --git a/src/ui_cm_interface.rs b/src/ui_cm_interface.rs index b62f59c54..1474ce093 100644 --- a/src/ui_cm_interface.rs +++ b/src/ui_cm_interface.rs @@ -377,6 +377,15 @@ pub fn close(id: i32) { }; } +/// Like `close`, but says the CM's WINDOW closed rather than a person disconnecting this peer. +/// See `ipc::Data::CmWindowClosed`. +#[cfg(target_os = "linux")] +pub fn close_window(id: i32) { + if let Some(client) = CLIENTS.read().unwrap().get(&id) { + allow_err!(client.tx.send(Data::CmWindowClosed)); + }; +} + #[inline] pub fn remove(id: i32) { CLIENTS.write().unwrap().remove(&id);