From ac96ccac85aca55938d5735b72251d63a73d2d21 Mon Sep 17 00:00:00 2001 From: rustdesk Date: Wed, 9 Sep 2026 16:14:23 +0800 Subject: [PATCH] review: keep the 30s watchdog hard, and let Android's picker hold the reconnect Timing the watchdog off receive progress gave away its upper bound. A fragment bumps the counter as it arrives, ahead of the framing checks that would reject it, so a peer sending one `FRAG_MORE` every twenty seconds and never a `FRAG_END` refreshed the deadline forever while the reassembly buffer grew toward `MAX_FRAME_LENGTH`, a gigabyte away. What it bought - a clipboard image that takes longer than thirty seconds to arrive is not a dead peer - is a pre-existing problem that predates this branch and can be fixed on its own. Receive progress goes back to the one job it was added for, which needs no deadline of its own: telling a transport that has gone quiet from one that is still delivering, so ICE's disconnected hint is not acted on mid-transfer. The Android document picker suppresses a `Connection Error` while it is open and remembers to reconnect once it closes. The peer-gone break reconnects under `restarting-show` with a `Connecting...` title, which matched neither half of that test, so an eight-second stall behind an open picker - Doze and background throttling produce them - threw a dialog up behind the picker and lost the deferred reconnect. It is now named there by its own title rather than by its type: an explicitly restarted remote device sends the same type from a path this leaves alone, on every transport, and deferring that one too would be a change to sessions this has no business touching. The two limits are still not hard upper bounds, and the comment saying so was wrong about why. A send is awaited inline in this loop, so one in progress delays the tick that checks them - bounded on WebRTC by the timeout the stream was built with, not bounded at all on KCP, whose framed stream is constructed with none. The 30s watchdog beside it shares the loop and the same delay. Left alone deliberately. `restarting-show` reconnects without the backoff its `restarting` sibling uses, which can loop while each round gets far enough to establish a session and then loses the transport within eight seconds; a cooldown there would also delay the recovery this exists for when a peer really does come back, and the loading it shows can be cancelled. And the KCP limit reads an accumulated silence rather than a transient hint, so unlike the WebRTC grace it needs no second sample to confirm - one would only move eight seconds to nine. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019aokqJuhjvB3kijXtAg5Ns --- flutter/lib/models/model.dart | 6 +++++- libs/hbb_common | 2 +- src/client/io_loop.rs | 17 +++++------------ 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/flutter/lib/models/model.dart b/flutter/lib/models/model.dart index 56c4462ca..c7a48280a 100644 --- a/flutter/lib/models/model.dart +++ b/flutter/lib/models/model.dart @@ -896,9 +896,13 @@ class FfiModel with ChangeNotifier { final text = evt['text']; final link = evt['link']; + // The peer-gone detector reconnects under `restarting-show` rather than an error title, so + // it needs naming here too. By its own title, not the type: an explicitly restarted remote + // device reaches the same type from a path this change does not touch. if (isAndroid && _androidDocumentPickerActive && - title == 'Connection Error') { + (title == 'Connection Error' || + (type == 'restarting-show' && title == 'Connecting...'))) { _androidDocumentPickerInterruptedConnection = true; return; } diff --git a/libs/hbb_common b/libs/hbb_common index 166bb3d2a..29cf7cbe4 160000 --- a/libs/hbb_common +++ b/libs/hbb_common @@ -1 +1 @@ -Subproject commit 166bb3d2ac3a59eeaafab6ac0aac7b98c8d93778 +Subproject commit 29cf7cbe4d38ce36020749f713fb066299f02431 diff --git a/src/client/io_loop.rs b/src/client/io_loop.rs index ab952dcdd..9de20588e 100644 --- a/src/client/io_loop.rs +++ b/src/client/io_loop.rs @@ -25,9 +25,6 @@ const WEBRTC_SUSPECT_GRACE: Duration = Duration::from_secs(3); // KCP gets no such hint, only how long since a packet arrived; its endpoint pings an idle peer // about every 2s, so this is several missed pings, and matches the 8s WebRTC arrives at. const KCP_PEER_SILENCE_LIMIT: Duration = Duration::from_secs(8); -// Neither is a hard upper bound: sends are awaited inline in this loop, so one in progress keeps -// the tick that checks them from running, capped only by the transport's own send timeout. -// Removing that needs the framing work that would stop one message owning the link. #[cfg(feature = "unix-file-copy-paste")] use crate::{clipboard::try_empty_clipboard_files, clipboard_file::unix_file_clip}; use base::{ @@ -262,7 +259,6 @@ impl Remote { let mut last_recv_time = Instant::now(); let mut webrtc_suspect_since: Option = None; let mut last_rx_progress = peer.rx_progress(); - let mut last_rx_progress_at = last_recv_time; let mut peer_gone = false; loop { @@ -309,11 +305,7 @@ impl Remote { self.handle_local_clipboard_msg(&mut peer, _msg).await; } _ = self.timer.tick() => { - // Not `last_recv_time` alone: a message larger than the transport's - // fragment size yields nothing until its last fragment, so a peer - // sending one steadily - a clipboard image is the case that occurs - - // would otherwise be timed out mid-transfer. - if last_recv_time.max(last_rx_progress_at).elapsed() >= SEC30 { + if last_recv_time.elapsed() >= SEC30 { self.handler.msgbox("error", "Connection Error", "Timeout", ""); break; } @@ -339,14 +331,15 @@ impl Remote { // given one, so they are inert here. let progressed = rx_progress != last_rx_progress; last_rx_progress = rx_progress; - if progressed { - last_rx_progress_at = Instant::now(); - } if peer.webrtc_disconnected() && !progressed { webrtc_suspect_since.get_or_insert_with(Instant::now); } else { webrtc_suspect_since = None; } + // Neither limit is a hard upper bound. A send is awaited inline in + // this loop, so one in progress delays this tick - bounded on WebRTC + // by the timeout the stream was built with, not bounded at all on + // KCP. The 30s watchdog above shares the loop and the same delay. peer_gone = webrtc_suspect_since .map_or(false, |since| since.elapsed() >= WEBRTC_SUSPECT_GRACE) || kcp