TheLanc3/rustdesk
1
0
Fork 0
mirror of https://github.com/rustdesk/rustdesk.git synced 2026-09-10 22:41:05 +03:00
Code Issues Packages Projects Releases Wiki Activity

Dead peer recovery (#16117)

Browse Source 
* webrtc: recover from a silent peer in about 8s instead of 30s

A controlled peer that is killed, switched away by a user switch, or rebooted
leaves no trace on a UDP transport: there is no reset to receive, so the session
sees silence, and only the 30s inactivity timeout ends it. By then the remote
machine may have finished rebooting and be reachable again, while the user has
been watching a frozen frame the whole time and is then told the peer reset the
connection.

ICE already knows sooner. It reports Disconnected about 5s after it stops
hearing from the peer, from its own task, so it stays accurate even while this
loop is busy sending. That state is transient by design - a Wi-Fi roam or a
sleep/wake recovers from it - so it is treated as suspicion, not as death: three
more seconds with the transport receiving nothing, and the session reconnects.
Receive progress cancels the suspicion, so a peer that is merely slow, or one
ICE was late to clear, is not dropped.

This only reaches the existing recovery sooner; it does not replace it. The
first reconnect goes out immediately and, if it fails, falls into the same
retry the UI already applies to any unexpected disconnect. The restart
reconnect event is reused deliberately: it is what asks for exactly that, with
no error dialog in front of it, and the UI shows "Connecting..." for it rather
than anything about restarting. Its five-minute grace stays reserved for a
restart the user actually asked for - silence is no evidence of a reboot.

The 30s timeout is unchanged and still backs every transport. TCP and WebSocket
are untouched. The controlled side is untouched: it detects a dead controller
on the same 30s, which wastes some capture but nothing a user sees.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019aokqJuhjvB3kijXtAg5Ns

* kcp: recover from a silent peer on the endpoint's own clock

KCP is the other transport with nothing to receive when the peer dies, and it
was the slower of the two: its endpoint reaps a connection only after 60s
without a packet, which is past the 30s inactivity timeout above it, so in
practice nothing but that timeout ever noticed.

The endpoint already tracks when each connection last heard from its peer and
now exposes it, so this reads that rather than anything derived from the session
loop - it keeps answering while that loop is busy sending. Its liveness ping now
goes out about every 2s rather than every 10s, so silence means the peer rather
than an idle link, and eight seconds of it is several missed pings.

Same threshold and the same recovery as the WebRTC half, so a user sees the same
thing on either transport.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019aokqJuhjvB3kijXtAg5Ns

* review: time the inactivity window off receive progress, bound the parting send

Two things the review found, both on the controlling side.

The 30s inactivity window still ran off completed messages alone, so the probe
added for the fast path did not fix what it was added for: a message larger than
the transport's fragment size yields nothing until its last fragment, and a peer
sending one steadily was still timed out mid-transfer. It is now timed off
whichever is later, a completed message or receive progress. Transports that
report no progress leave that at its starting value, so nothing else moves.

The parting close-reason send for KCP waited on send capacity with no deadline
of its own, and a queue a dead peer will never drain held the finished session's
thread until the endpoint reaped the connection a minute later. Bounded once the
peer has been declared gone. Still attempted rather than skipped: if the loss was
one-way the peer does receive it, and drops its side immediately instead of
waiting out its own timeout - which is also the one case where the note below
resolves itself.

Recorded from the same review, for the case none of this targets - a peer that
is alive behind a path that broke for five to ten seconds and then healed.
Giving up cannot deliver a close there, because the path is still down at that
moment, so the controlled side keeps the old connection until its own 30s
expires. For up to twenty of those seconds it holds two authorised connections:
its connection manager lists both, and the stale one reports a growing delay
that pins the shared frame rate low for the new one. Input is unaffected
throughout and both recover once the stale connection goes, so this trades
twenty-two seconds of a frozen, uncontrollable session for a controllable one
that looks wrong for a while. Closing the displaced connection is controlled-side
work and belongs with the rest of it, not here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019aokqJuhjvB3kijXtAg5Ns

* review: reject a disconnected cached session, tidy the detector

hbb_common: `is_reusable_for` now also rejects a session ICE reports
Disconnected, so a caller is not handed one that already carries the hint; and
the receive-progress test no longer races `next()` against a sleeping sibling.

Here: the `is_some()` guard on the progress comparison was dead, since a
transport answers `None` for its whole life and `None != None` is already false.
The parting-send deadline is a `Duration` like every other constant around it
rather than bare milliseconds. And the comments are cut back to what is not
already evident from the code they sit on.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019aokqJuhjvB3kijXtAg5Ns

* review: keep the legacy UI's retrying error when the peer goes silent

`restarting-show` is a Flutter control event; Sciter has no case for it and
falls through to a plain dialog, which `check_if_retry` marks non-retryable
because its type is not `error`. So on that build the new detector would have
replaced a timeout that reconnects on its own after 30s with a dialog waiting
for a click at 8s - a regression for the one path this was meant to shorten.

Send it the message the timeout already sends, so its behaviour is unchanged
apart from arriving sooner.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019aokqJuhjvB3kijXtAg5Ns

* 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019aokqJuhjvB3kijXtAg5Ns

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
RustDesk
2026-09-09 17:56:41 +08:00
committed by GitHub
parent 080211ff36
commit f164c9a9df
5 changed files with 71 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
Cargo.lock generated
View File

@@ -4280,7 +4280,7 @@ dependencies = [
[[package]]
name = "kcp-sys"
version = "0.1.0"
source = "git+https://github.com/rustdesk-org/kcp-sys?branch=rustdesk-patches#023a0065398968989f2ddfcf5cc72bb886d02675"
source = "git+https://github.com/rustdesk-org/kcp-sys?branch=rustdesk-patches#938eda3e5e9757a612385503af7a6cb1189b2cdd"
dependencies = [
"anyhow",
"auto_impl",

6
flutter/lib/models/model.dart
View File

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

2
libs/hbb_common

Submodule libs/hbb_common updated: 55395c6fcb...29cf7cbe4d

49
src/client/io_loop.rs
View File

@@ -15,6 +15,16 @@ use crate::{
// Restart msgbox text is kept as a legacy UI fallback; Flutter handles the type as a control event.
const RESTART_REMOTE_DEVICE_NO_DATA_TIMEOUT: Duration = Duration::from_secs(5);
const KCP_CLOSE_REASON_FLUSH_DELAY: Duration = Duration::from_millis(30);
// Deadline for the parting close-reason send once the peer is presumed gone; KCP waits for send
// capacity with no deadline of its own.
const KCP_CLOSE_REASON_GONE_DEADLINE: Duration = Duration::from_millis(500);
// Grace after ICE reports Disconnected, which it does ~5s after it stops hearing from the peer,
// for ~8s in total. Disconnected is transient by design, so this waits out a Wi-Fi roam or a
// sleep/wake rather than acting on the first hint.
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);
#[cfg(feature = "unix-file-copy-paste")]
use crate::{clipboard::try_empty_clipboard_files, clipboard_file::unix_file_clip};
use base::{
@@ -247,6 +257,9 @@ impl<T: InvokeUiSession> Remote<T> {
let _keep_it = client::hc_connection(feedback, rendezvous_server, token).await;
let mut last_recv_time = Instant::now();
let mut webrtc_suspect_since: Option<Instant> = None;
let mut last_rx_progress = peer.rx_progress();
let mut peer_gone = false;
loop {
tokio::select! {
@@ -313,6 +326,37 @@ impl<T: InvokeUiSession> Remote<T> {
self.handler.msgbox("restarting-show", "Restarting remote device", "Connection in progress. Please wait.", "");
break;
}
let rx_progress = peer.rx_progress();
// `None` for transports that report none, and it never changes for a
// given one, so they are inert here.
let progressed = rx_progress != last_rx_progress;
last_rx_progress = rx_progress;
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
.as_ref()
.and_then(|k| k.peer_silent_for())
.map_or(false, |silent| silent >= KCP_PEER_SILENCE_LIMIT);
if peer_gone {
log::info!("Peer stopped answering, reconnecting");
#[cfg(feature = "flutter")]
self.handler.msgbox("restarting-show", "Connecting...", "Connection in progress. Please wait.", "");
// Sciter knows no `restarting-show` and would show a dialog that
// waits for a click, where the timeout this arrives ahead of is
// retryable and reconnects on its own. Keep that message for it.
#[cfg(not(feature = "flutter"))]
self.handler.msgbox("error", "Connection Error", "Timeout", "");
break;
}
let elapsed = fps_instant.elapsed().as_millis();
if elapsed < 1000 {
continue;
@@ -358,6 +402,11 @@ impl<T: InvokeUiSession> Remote<T> {
s.send(()).ok();
}
if kcp.is_some() {
// Attempted rather than skipped even here: if the loss was one-way the peer
// does get it, and drops its side instead of waiting out its own timeout.
if peer_gone {
peer.set_send_timeout(KCP_CLOSE_REASON_GONE_DEADLINE.as_millis() as u64);
}
// Send the close reason if it hasn't been sent yet, as KCP cannot detect the socket close event.
self.send_close_reason(&mut peer, "kcp").await;
// KCP does not send messages immediately, so wait to ensure the last message is sent.

19
src/kcp_stream.rs
View File

@@ -8,14 +8,15 @@ use hbb_common::{
tokio_util, ResultType, Stream,
};
use kcp_sys::{
endpoint::KcpEndpoint,
endpoint::{ConnId, KcpEndpoint},
packet_def::{KcpPacket, KcpPacketHeader},
stream,
};
use std::{net::SocketAddr, sync::Arc};
pub struct KcpStream {
_endpoint: KcpEndpoint,
endpoint: KcpEndpoint,
conn_id: ConnId,
stop_sender: Option<oneshot::Sender<()>>,
}
@@ -41,6 +42,14 @@ impl KcpStream {
}
}
/// How long since a valid packet was last received from the peer, or `None` once the
/// connection is gone. Answered by the KCP endpoint's own tasks, not by the session's read
/// loop, so it stays meaningful while that loop is busy sending a large message; and the
/// endpoint pings an idle peer often enough that silence here means the peer, not quiet.
pub fn peer_silent_for(&self) -> Option<std::time::Duration> {
self.endpoint.peer_silent_for(&self.conn_id)
}
fn create_framed(stream: stream::KcpStream, local_addr: Option<SocketAddr>) -> Stream {
Stream::Tcp(FramedStream(
tokio_util::codec::Framed::new(DynTcpStream(Box::new(stream)), BytesCodec::new()),
@@ -77,7 +86,8 @@ impl KcpStream {
if let Some(stream) = stream::KcpStream::new(&endpoint, conn_id) {
Ok((
Self {
_endpoint: endpoint,
endpoint,
conn_id,
stop_sender: Some(stop_sender),
},
Self::create_framed(stream, udp_socket.local_addr().ok()),
@@ -108,7 +118,8 @@ impl KcpStream {
if let Some(stream) = stream::KcpStream::new(&endpoint, conn_id) {
Ok((
Self {
_endpoint: endpoint,
endpoint,
conn_id,
stop_sender: Some(stop_sender),
},
Self::create_framed(stream, udp_socket.local_addr().ok()),