The controller sends every candidate twice, because the server's hop to a
peer registered over UDP can lose one. The ICE agent that dedups repeats
sits downstream of the answerer's queue, so the answerer paid for both
copies: a slot, a JSON parse, and the ICE agent's lock, once per repeat.
Remember a digest of what was queued and skip the repeat. Recorded only
once queued, so a candidate a full queue refused stays repairable by the
re-send.
The queue's depth is unchanged. A real peer gathers well under it - four
STUN servers, link-local IPv6 filtered, one component - and the drain
empties it as candidates trickle in, so what this removes is the redundant
work, not an overflow.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019aokqJuhjvB3kijXtAg5Ns
The earlier commit took dcsctp's min_rtt_variance = 220 as a raw floor under
rttvar. dcsctp divides the option by kHeuristicVarianceAdjustment = 8.0 first,
a historical accident it kept because downstream users had measured good
values with it, so the intended floor is 27.5ms of variance contributing 110ms
to RTO. Flooring at 220 contributed 880ms instead, which on a 50ms path left
RTO within 7% of the 1000ms default this change exists to escape.
The fork also now records why T1/T2 share T3's RTO manager here, unlike
dcsctp's separate control timers: RTO_INITIAL is the T3 value for the first
DATA chunk, since no RTT sample exists before the first SACK.
punch_udp sent a zero-length datagram and called the hole open on whatever
arrived next. The rendezvous NAT test's own leftover replies satisfy that
immediately - connect() does not flush the receive queue - so the retry loop
never ran and success meant nothing. The dead socket then cost KCP its full
timeout to rediscover, which is how a failed punch came to take 18 seconds.
Probes now carry a magic and a 64-bit transaction id, and both ends answer
each other's probes, so returning is a fact: a reply echoing our own id is the
one thing that proves the pair carries traffic both ways. With failure now
distinguishable from 'not yet', the window drops from 20s to 3s.
Two asymmetries fall out of that:
Only the connector stops on its own acknowledgement, because only it has
something to send next. An acknowledgement proves our probe came back, not
that the peer's probe was answered - and after punch_udp returns nothing
answers probes any more, since KCP's io loop drops anything shorter than its
header. A listener that stopped there would go mute while a peer whose own
probe or answer was lost - the normal state of a hole still opening - kept
probing an endpoint that works, until it timed out.
So the listener stops on the peer's first real packet instead, and hands that
packet to KcpStream::accept as its init_packet: its arrival proves the pair as
well as an acknowledgement would, and KCP never retransmits its SYN.
webrtc-sctp ships RFC 4960's RTO.Initial/RTO.Min (3000/1000), TCP's values for
arbitrary public paths. On this workload they set the recovery time outright:
a request/response exchange keeps one chunk in flight, so no later SACK ever
raises miss_indicator to the 3 that arms fast retransmit, and the T3 floor is
the only way back. A single loss during a handshake or a first keyframe
therefore costs whole seconds.
The fork now carries dcsctp's numbers instead - the SCTP implementation Google
wrote to replace usrsctp for Chrome's WebRTC data channels, the same realtime
workload: rto_initial 500, rto_min 400, a 220ms floor under the RTT variance,
and mtu 1191. INITIAL_MTU 1228 plus DTLS/UDP/IPv6 overhead is 1313, past the
1280 minimum, so every full-size chunk fragmented on an IPv6 path.
Both patch entries move to the new branch, which also carries the Windows IPv6
byte-swap fix, so one rev matches the whole webrtc 0.13 stack.
`test_ipv6` kept its own hand-written copy of the STUN servers. It now reads
`WebRTCStream::stun_servers()`, so an operator who points OPTION_ICE_SERVERS at their own
server gets it on both paths instead of one.
`test_bind_ipv6` sends nothing - `connect` only makes the kernel pick a route and a source
address - so the whole cost is DNS. It races the lookups rather than betting this host's
IPv6 support on whether the first entry happens to publish a AAAA where the user resolves
from; google's does not, from a Chinese resolver, and it was the entry being bet on.
`stun_ipv4_test`, `STUNS_V4` and `test_nat_ipv4` have had no callers since the punch stopped
taking its port from a second socket, and go.
`get_kcp_cc_enabled` reads the renamed option through `option2bool`, like every other one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019UzcMTdYTEv2QbMHcTSUy3
`stream_type` reaches the UI as the transport that won the race, and every other transport
already carries the family in that label - the v6 punch reports `IPv6`. WebRTC does not: one
label covers both families, and it is the one path whose real remote address can differ from
the rendezvous-observed one the session is identified by.
Refine it at the hand-off to the UI rather than at the source: five sites in client.rs
compare `typ == "WebRTC"`, so widening the label there would silently move control flow.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019UzcMTdYTEv2QbMHcTSUy3
Also pin webrtc-util to a fork of 0.11.0 carrying a Windows IPv6 enumeration fix.
`ifaces` reads the adapter list's on-wire IPv6 bytes as host-order `[u16; 8]`, so on a
little-endian host every group comes out byte-swapped and unbindable: a peer's real
240e:369:9606:4600:f52a:7a8d:2530:4de0 is enumerated as e24:6903:696:46:2af5:8d7a:3025:e04d,
::1 as ::100 and fe80:: as 80fe::. Each fails to bind with WSAEADDRNOTAVAIL, so ICE gathers
no IPv6 host candidate at all on Windows - where a globally routable address is the one
NAT-free path a CGNAT'd peer has.
Never reported upstream; the unix twin of the same bug was fixed in webrtc-rs#475 (2023).
Fork: rustdesk-org/webrtc, branch rustdesk-patches, tag webrtc-util-0.11.0-win-ipv6.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019UzcMTdYTEv2QbMHcTSUy3
The zero-timeout cancellation in `test_cancelled_new_does_not_leak_the_pc`
is not guaranteed to win — the setup task can finish inside the single
poll it allows, and `new()` then returns a live stream that the test
discarded. `WebRTCStream` has no `Drop`, so that stranded its own pc in
SESSIONS and the test reported it as the cancelled attempt's leak.
Nothing in production was leaking. 24 tests now pass at 1, 2, 4, 8 and
default thread counts; `--test-threads=2` had failed every run.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019UzcMTdYTEv2QbMHcTSUy3
The leak test waited for an instant with no new keys, which needs the
whole suite idle; it now watches for a key that outlasts its window,
which is what "leaked" means. `--test-threads=2` still fails: a real
entry survives the wait, and it is not the one this test creates.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019UzcMTdYTEv2QbMHcTSUy3
`get_local_endpoint_trickle` became `local_endpoint() -> &str`, which
cannot fail, so both call sites lose an unreachable error arm — the
mediator's closed a pc against a failure that no longer exists.
`punch_type` named one transport, and picked it off `allow_tcp_punch`.
A round carries several at once — a NAT port and a v6 address and an
offer — and since the TCP punch became a switch it can carry none, so
one name had to misreport both: the logs of the round that broke WebRTC
read "#1 UDP punch attempt" while the request also carried the v6
address and the offer that was actually failing, and a round with
nothing to punch with was labelled "WebRTC". List them instead —
"UDP+IPv6+WebRTC" — and call the empty round "Relay", which is what it
can still end as and what `typ` prints for it.
The offer is moved into the request rather than cloned into it; that
was its last use.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019UzcMTdYTEv2QbMHcTSUy3
The offer the punch request carries grew with ICE gathering, so the
rendezvous server's `PunchHole` datagram to a UDP-registered peer
fragmented and was dropped silently. The trickle endpoint is now taken
once at construction and carries no candidates — a fixed 673 bytes.
Both sides need this: the answer travels the same encoding path, and on
the UDP-punch route it rides a datagram of its own.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019UzcMTdYTEv2QbMHcTSUy3
TCP punching was the one direct transport without a switch, while UDP,
IPv6 and WebRTC each had one. Add "Enable TCP hole punching" above the
UDP toggle on both desktop and mobile, default on — including on
self-hosted servers, since unlike the other three (whose default-off
there guards against an hbbs that cannot forward their fields) TCP
punching has always been supported by every server.
Turning all four off would leave no way to punch at all, so TCP runs
regardless in that case. That backstop keys off the switches alone: a
transport that is enabled but fails to materialize — no public v6
address, no NAT port, a failed offerer — is already covered by the
relay fallback for a round that ends up with no usable direct
transport. With the TCP punch off, the fallback request is skipped
too: it exists only to carry that punch, and would otherwise reach
connect() with nothing to try and merely open a second relay.
Known cost, unchanged behavior for the peer: the request carries no
field for this choice, so a peer that receives one with no udp_port and
no offer still punches a TCP hole and listens for a connection the
controller will not make. Representing the transport choice on the
wire needs a proto field and the server forwarding it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016HV43uh1ztv6Wm5qi3Y1ne
The punch request carries udp_port only if the rendezvous server's
TestNatResponse has arrived, and the wait for it was bounded by
rtt / 2 — half the TCP connect time, on the assumption that TCP and
UDP round trips are comparable and the test, started earlier, has
already answered.
A transparent TCP proxy breaks that assumption: a TUN-mode VPN on the
host, or a redirect-mode proxy on the LAN gateway serving every device
behind it, completes the handshake locally in ~3ms while the real UDP
round trip is hundreds of ms. Log-confirmed against 5.161.65.208: ping
341ms, TCP connect 3.7ms, connect to a dead port there "succeeds" just
as fast. The window collapsed to ~1.5ms, udp_port stayed 0 on every
attempt, and UDP punch was never even requested — although UDP itself
passes such gateways untouched.
So use the TCP clock only when it is believable: below a plausible WAN
round trip it says nothing about the UDP path, and a flat ceiling
applies instead. The loop still exits the moment the port arrives, so
a genuinely nearby server pays nothing and only a UDP-dead network
waits out the ceiling — on the udp-carrying round alone, while the
parallel pure-TCP round is unaffected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016HV43uh1ztv6Wm5qi3Y1ne
Closing the controlling window left the controlled side waiting out
ICE decay — ~25-30s in the peer's log, its disconnected/failed ladder
running to completion — where TCP delivers a FIN at once. The session
end closed the pc by spawning onto io_loop's own
`#[tokio::main(flavor = "current_thread")]` runtime, which is dropped
the moment io_loop returns, and nothing after that call yields: the
task was never polled even once, so no DTLS close_notify ever left.
Every attempt to fix that on the caller's side failed the same way,
because the mismatch was never about where the close ran: a pc's UDP
sockets register with the reactor, and its ICE/DTLS/SCTP pumps spawn
on the runtime, that is current while it is built — so a pc created
by a session outlives the only runtime that can drive its I/O, and a
close driven anywhere else completes without reaching the wire.
The bump homes them where they can outlive any caller: WebRTCStream
builds on a process-lifetime runtime and every detached close runs
there as its own never-cancelled task. io_loop keeps its plain
close_webrtc() calls and only documents why nothing here may spawn or
await the teardown on the dying session runtime.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016HV43uh1ztv6Wm5qi3Y1ne
Three correctness fixes in the transport race, plus three convention
cleanups.
- race_transports_prefer_webrtc committed a relayed result while a direct
attempt was still in flight: the others arm returned on
webrtc_fut.is_none() even with an unfinished direct future, and the
WebRTC-error arm returned a held relay without checking others_fut. A
relay is now committed only when nothing direct can still arrive (or
the window expires); a parked relay is also preferred over composing
an error when both sides fail. Three regression tests, mutation-checked.
- connect()'s plain select_ok let a TURN-relayed WebRTC win as "first
success", dropping still-racing UDP/IPv6 direct attempts and reporting
the relayed pair as direct. It now runs through the same prefer-P2P
race with each attempt carrying whether its path is direct, and the
WebRTC future resolves is_relayed() so a TURN win is held behind
direct attempts, not committed as one.
- The RelayResponse path kept direct == true when a WebRTC win's DTLS
handshake failed and it fell back to relay, so the relay was reported
P2P. Clear the flag with the transport switch.
- Trim the OffererGuard doc to the three-line max; move the new
enable-webrtc localization key to the end of every lang list; the KCP
option constant moved to hbb_common config::keys (0f663aa).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
386 added comment lines down to 287 across client, mediator, kcp_stream
and common. Same rule as hbb_common 3d64e43: out go past-bug narration,
rejected alternatives, measurements and restatements of the code; the
non-derivable why stays.
is_direct_transport goes with them. Judging the race by a transport
label was replaced by the resolved direct flag, leaving it used only by
its own test — and, having been inserted between the doc comment and
race_transports_prefer_webrtc, it had also taken that function's
contract with it. Removing it reattaches the doc where it belongs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
Third review round. Two of these are regressions from the previous one.
- The RelayResponse race predicate was `is_direct_transport(result.2)`,
which answers true for the label "WebRTC" - but WebRTC is only a
direct path when ICE nominated a non-TURN pair. A TURN-relayed WebRTC
result therefore committed instantly and cancelled the IPv6 attempt
racing beside it, which is the same inversion the previous fix removed
in the other direction. (That fix was also argued from a wrong premise:
the site does carry an IPv6 future, pushed ~50 lines earlier than the
relay one.) Each future now resolves whether its path is direct and
the predicate reads that bool, matching the outer race, and the
downstream recomputation goes away.
- policy_relay still folded in Config::is_proxy(), and that is what gets
persisted into the peer's config as force-always-relay - so one
session through a proxy pinned the peer to relay forever and disabled
WebRTC for it, exactly the latch the previous round fixed for
WebSocket. Split out peer_relay: the saved option or an explicit
request for THIS peer, and the only part written back.
- The controlled side buffered remote ICE candidates in an unbounded
channel while the controller caps the same buffer at 64, and draining
one costs a JSON parse plus the ICE agent's lock. Whoever can reach a
session's route could grow it without limit inside the long-lived
service process. Bounded, with the overflow logged through the
existing throttle.
- That route was also removed by key alone when an answerer finished, so
a punch retry that built a fresh answerer under the same fingerprint
had its live sender deleted by the previous one's cleanup - after
which it received no candidates at all. Evict only our own sender, the
way the session cache already guards the analogous case.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
The webrtc feature pulls time 0.3 into scrap's graph (hbb_common ->
webrtc -> webrtc-dtls -> der-parser -> asn1-rs), and that crate carries
an `impl Div<time::Duration> for std::time::Duration`. Orphan rules
allow it because the RHS is its own type, and trait impls are visible
across the whole dependency graph without a use, so std::time::Duration
now has two Div candidates. `yuv_count as _` casts to a plain inference
variable, which both candidates fit, so it stops resolving:
error[E0282]: type annotations needed
--> libs/scrap/examples/benchmark.rs:146:33
Only two of the four sites are reported - rustc emits one E0282 per
function body - so all four are annotated. The already-explicit
`as u32` at the hwcodec site and `start.elapsed() / cnt` are unaffected,
the latter because an integer literal's variable can only unify with an
integral type and rules the time impl out on its own.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- close_webrtc is no longer async (hbb_common 88f965f), so the ten call
sites in port_forward and io_loop - all inside select! arms or futures
the UI can abandon - can no longer be cancelled mid-teardown, which
left the pc unclosable and its session entry stranded. Client's own
spawn_close_webrtc went with it: the runtime-teardown guard it existed
for now lives in close_detached, so both Drop paths share one
implementation.
- webrtc_relayed() returns None when no candidate pair is selected or
the pc closed under a concurrent teardown, and both call sites read
that as "not relayed", i.e. direct. A TURN-relayed session could
therefore be shown to the user as peer-to-peer. Claiming a direct path
needs evidence of one, so an unknown answer now counts as relayed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
Review of #15684 and hbb_common#579. Each of these left the code reading
correct while the feature did not function.
- The RelayResponse race classified P2P with `result.2 == "IPv6"`, but
that site's futures are only ever the relay ("Relay"/"WebSocket") and
the WebRTC branch's own "WebRTC" — so the predicate was constantly
false. When the relay landed first the result was still right (the
webrtc arm's `others_fut.is_none()` fallback), but when WebRTC
connected FIRST it was parked as if it were a relay and the relay was
committed on arrival, discarding a live direct connection. That is the
LAN case: the better the network, the worse the outcome. Classify by
what the label means, via is_direct_transport, and test both orderings
— only the relay-first one was covered.
- handle_peer_info wrote "force-always-relay=Y" into the peer's saved
config whenever force_relay was set, which now includes the WebSocket
transport. One ws session therefore turned the peer into a permanent
relay-by-policy peer, and relay-by-policy means Relay-only ICE, so
WebRTC could never go direct to it again — the flagship path worked
exactly once. Persist policy_relay, which is the user's choice; the
transport is a property of this client, not of the peer.
- The answerer gated on this machine's enable-webrtc option, but that is
LocalConfig: the UI process writes it and never syncs it over IPC,
while handle_punch_hole runs in the server process, which on Windows
resolves LocalConfig under a different profile and reads the
private-server default of "N". The gate refused to answer in exactly
the self-hosted deployments the transport exists for. Drop it: the
answerer follows the request, like the udp/ipv6 legs, and the option
still gates the feature where it can — an offer only exists because
some controller had it enabled.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
Turning on hbb_common's "webrtc" feature pulls webrtc-util into the android
link, and its ifaces() -- reached from vnet::Net::new() on every ICE gather --
calls getifaddrs(). bionic exports getifaddrs/freeifaddrs only from API 24,
while flutter/ndk_*.sh builds against --platform 21, so every abi failed to
link on the undefined symbols.
Raising the platform to 24 would have to drag minSdkVersion 22 with it and
turn the link error into a load-time one on Android 5.1/6.0, so define the
two symbols instead, using the RTM_GETLINK + RTM_GETADDR netlink dump bionic
itself uses. The definition also shadows bionic's on API >= 24 rather than
delegating to it, so the path that ships is the path every test device runs.
Checked against synthesised netlink dumps on the host -- link/address parsing,
prefix masks, point-to-point, ipv6 scope ids, malformed and truncated messages
-- under UBSan and byte-exact guard malloc, with a deliberately unsigned
remainder as the negative control.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The branch had flipped KCP to nc=0 (built-in congestion window) for
every session. That is a transport-behavior change for all users made on
reasoning alone, and the reasoning does not decide it: which profile wins
depends on why packets are being lost.
nc=1 - what RustDesk has always shipped - never shrinks the send window,
so on a genuinely congested uplink it deepens the loss it is reacting to.
But nc=0's backoff is blunt: a fast retransmit halves the window while an
RTO sets cwnd = 1 outright (ikcp.c) and recovery slow-starts from one
packet, so on a link with random loss and no congestion - Wi-Fi
interference, a long-haul path - it reads loss as congestion and can
stall an interactive stream for seconds. That failure mode is also the
more visible one to a remote-desktop user.
No benchmark settles this either: a loopback A/B has no bottleneck queue,
hence no congestion to control, and would flatter nc=1 by construction.
Deciding it needs a shaped link or field data.
So keep the profile users already run and let the other one be asked for
("enable-kcp-congestion-control" = "Y"). Flipping the default later is a
one-line change once there is evidence. kcp-sys keeps its own test
covering the nc=0 path.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
auth.rs predated hbb_common's LogThrottle and grew its own equivalent:
same shape (last_log_at + suppressed), same 5s interval, plus a helper
and three OnceLock<Mutex<..>> statics. It also counted the other way -
excluding the event being reported - so each of the three sites carried
two near-identical log::warn! arms to avoid printing "suppressed 0".
The shared macro covers all of it: one static per call site declared by
the expansion, and the multiplicity suffix appears only when there is
one, which is what those duplicated arms were for. 102 lines out, 27 in.
Behavior difference, deliberate: a burst now reads "(x47)" - the total
including this line - instead of "(suppressed 46 similar events)". One
number, no arithmetic, and one convention across the codebase.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
kcp-sys has been through two review rounds of behavioral fixes; the
client wrapper (kcp_io pumps, connect/accept deadlines, framed-stream
adaptation, guard lifetimes) had no tests pinning what rustdesk actually
relies on. Four now do, each through real 127.0.0.1 UDP sockets and the
BytesCodec framing sessions use:
- handshake + bidirectional framed roundtrip + graceful close: the peer
observes end-of-stream instead of hanging (guard outlives the framed
stream so the FIN goes out);
- a writer that queues 50 frames and closes immediately loses none of
them - the client-side pin for the close-tail-drain semantics;
- socket errors after the peer vanishes are treated as loss: writes keep
succeeding, nothing tears down (ICMP is advisory on connected UDP);
- the connect deadline holds when nothing answers.
Mutation-checked: dropping inbound forwarding in kcp_io reddens exactly
the three tests that need the pump, and the timeout test alone stays
green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
OPTION_ENABLE_WEBRTC (hbb_common 48c2d4d) follows the udp/ipv6 punch
options end to end: default on against the public server, off against
private ones, same settings UI placement on desktop and mobile, and the
same bool2option local-option handling. Gates:
- controller: should_create_webrtc_offerer checks it first — no pc, no
STUN/TURN gathering, no offer in the request;
- controlled: unlike the udp/ipv6 legs, which deliberately follow the
request, answering builds a pc that gathers ICE from this host, so
the answerer honors this machine's own switch too.
Translations for "Enable WebRTC P2P connection" added to all 50 lang
files next to the IPv6 entry (IPv6 and WebRTC are invariant terms in
the same grammatical slot in every one of them).
Also stop probing v6 reachability (test_ipv6) under any forced relay:
the v6 punch socket is never bound there, so the probe was wasted work
on every ws/proxy/relay connection.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
Companion to hbb_common 68d2729: the full-ICE declaration now lives as
an `ice_policy: "all"` key inside the webrtc:// envelope, so the request
assembly no longer sets webrtc_all_ice and the controlled side asks the
envelope (endpoint_declares_all_ice) instead of a PunchHole field. The
rendezvous server carries the offer opaquely — no forwarding to keep in
sync. Skew behavior is unchanged: an unmarked or unparseable envelope
reads as the old Relay-only semantics.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
Reverts the connect/accept/add_conn changes that regressed concurrent
connects (the state_map guard held across add_conn is load-bearing), states
the single-conn contract on KcpEndpoint so shared-endpoint behaviour stops
consuming review effort, pins the two invariants that keep truncated input
from aborting under panic='abort', and fixes three findings from external
review: sendwnd() echoing raw config instead of KCP's effective window (a
non-positive factory value stalled sending forever), the passive closer's
lost final FIN delaying EOF by up to ~20s, and the doubled window
overflowing for extreme factory values.
Lock-only change: cargo update -p kcp-sys also re-picked libloading's
windows-targets between two versions already present in the lock; that was
reverted to keep this commit to the one line it is about. cargo metadata
--locked passes on the result.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
WebSocket support folds into force_relay because a ws tunnel kills
classic TCP/UDP punching — but that conflated transport necessity with
relay policy, and the WebRTC decisions keyed off the merged flag: a ws
client built no offerer at all without TURN, and only a Relay-only-ICE
one with it. ws deployments could never reach a direct WebRTC
connection, which is exactly the path they are supposed to live on.
Split the flag. LoginConfigHandler now tracks policy_relay (the
force-always-relay option, an explicit relay request — /r ids and
retry-via-relay included — and proxy) separately; force_relay stays
policy_relay || use_ws() and keeps governing the classic paths, so
non-ws behavior is unchanged everywhere:
- the offerer's existence and ICE policy follow policy_relay: under
pure ws the offer gathers every candidate type and may go direct;
under relay-by-policy it stays Relay-only ICE, TURN-gated, exactly
as before;
- the RelayResponse race applies the prefer-P2P window under ws (a
direct ICE path is worth delaying an already-ready relay for) while
policy relay keeps first-success semantics;
- the request carries webrtc_all_ice (hbb_common 64b54ab) so the
controlled side knows the offer is full-ICE: it answers with full ICE
and no TURN requirement, while offers without the bit keep today's
relay-only answer path on every version-skew combination.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
Picks up the handshake-recovery work plus the review round on top of it:
ABBA deadlock between the endpoint's two DashMaps, graceful-close tail
truncation, mid-stream hole on ikcp_send failure, FIN retransmission for
lost-FIN half-open hangs, SYN-ACK budget burned on dropped packets,
spurious ConnectTimeout after a completed handshake, accept-backlog
overflow stranding conns, aliasing UB in the output callback, and the
log-facade/throttling cleanup (per-packet sites no longer reach the
debug-level file logger, peer-rate warns throttled).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
9ea5442..cdcfd8d. `requester_id = 11` never reached main or hbbs, so nothing has
written or read that tag and reserving it guarded a wire format that never
existed — inconsistent with this branch retyping IceCandidate's tag 2 in place.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
b7f79c6..9ea5442 — reject a fragment header that is neither FRAG_END nor
FRAG_MORE, and a FRAG_MORE carrying no payload. The latter is the one nothing
downstream caught: it adds nothing to the reassembly accumulator, so the
MAX_FRAME_LENGTH cap never trips and WebRTCStream::next() spins for as long as
the peer keeps writing, with no error and no teardown.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The send and recv arms shared one counter, and an ICMP error on a connected
socket is reported once and then cleared — so the steady state is an
alternation: the send succeeds and clears the counter, the next recv reports
the error and finds the counter at 1, and logs. Every error still wrote a
line, at the ~100/s the previous commit set out to stop, while the
persistent-failure and recovery branches were unreachable.
Use one LogThrottle per direction instead of a hand-rolled counter. That
removes the shared state the bug lived in, drops a third throttling mechanism
in favour of the one already added, and leaves the surrounding `if let Err`
untouched rather than reshaping it into a match.
Also fix test_udp_uat's socket-error arm, the untreated twin of the punch_udp
site: it had no backoff at all, so a persistent error re-armed recv
immediately and spun the loop at CPU speed, one warn line per iteration.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
Debug output goes to the log file, so a site that fires per received message
or per retry lets someone else decide how much a machine writes to disk. The
WebRTC work added the first such sites.
- KCP io loop: absorbing ICMP errors as packet loss made a broken socket write
~100 lines a second for the 60s until the pong timeout reaps it. Log by run
instead: one line when a run starts, one per ~5s while it persists so a stuck
socket stays visible, and one on recovery with the total.
- punch_udp: the recv error retries every 10ms for up to MAX_TIME, so one line
per occurrence wrote thousands per punch. Log the first, report the count in
the timeout message.
- ICE candidate paths (client, mediator): the peer sets the candidate rate and
the rendezvous route carrying them needs no prior punch, so throttle to one
line a minute each with the suppressed count.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
Candidates arrive in gathering order — host, then srflx, then relay — so a
full buffer was discarding exactly the ones that traverse NAT while keeping
host ones that only work on a shared LAN. Evict from the front instead.
Also document why the controller's ICE bridge must not reconnect on error, in
contrast to the controlled side's per-candidate retry: its socket address is
the return route itself (mangled into PunchHole.socket_addr, echoed back in
IceCandidate.socket_addr, resolved through tcp_punch), so a reconnect would
arrive from an address no route points at, and the server drops the old entry
when the connection closes. Once it dies both directions are dead, and
abandoning WebRTC is the correct response rather than retrying.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
race_transports_prefer_webrtc committed any success from its first argument
outright, on the assumption that it is the WebRTC connect. It is not: the call
site passes a whole punch attempt, which internally falls back to request_relay
when its direct transports fail. That relay was therefore committed instantly
while the offer-less fallback's TCP punch was still in flight — inverting the
preference this function exists to enforce, since the is_p2p predicate the
caller already supplies was applied only to the `others` branch.
Apply it to both branches: a direct result from either side still commits
immediately, and a relayed result from either side is held for the window so
the other side can land something direct. Also commit a held connection when
the surviving branch errors, which the previous code only did on the first
branch's failure path.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
The rebase onto master (switch-code feature) added an 8th request_relay
parameter; pass the interface's switch code from both WebRTC->relay
fallback paths so a role-swap session survives the fallback. Also drop
a duplicate bindgen 0.72.1 entry the Cargo.lock merge produced.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
- treat ICMP-driven UDP socket errors (WSAECONNRESET 10054 on Windows,
ECONNREFUSED on Linux) as packet loss in punch_udp and the KCP pump
instead of tearing the session down; KCP retransmits through them and a
truly dead link is still reaped by the pong/app-level timeouts
- resolve STUN hostnames via tokio::net::lookup_host so DNS never blocks a
runtime worker; fix the inverted non-IPv4 error message
- add enable-kcp-congestion-control option (default on): switch the turbo
profile to nc=0 so brief loss on constrained links no longer spirals into
stalls; sender-side only, no wire negotiation
- pin kcp-sys to the rustdesk-patches branch: upstream main lost the
RustDesk patches on the EasyTier sync, and this branch also wires
set_kcp_config_factory into connection setup, making the option effective
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- the WebRTC offer now rides any punch request; only an offer-less request
may close and reuse the rendezvous socket for TCP punching
(request_allows_tcp_punch replaces the udp_port-based invariant), with a
separate offer-less request racing as the TCP fallback
- WebSocket mode no longer disables WebRTC — ws only tunnels the
signaling/relay legs while ICE stays the only P2P path there; SOCKS proxy
still disables it (ICE would bypass the proxy and leak the real IP)
- controlled side: WebRTC-only punch replies and trickled ICE candidates go
over dedicated TCP connections to the rendezvous server instead of the UDP
mediator channel, for ws/TCP-only hbbs deployments; drop the now-redundant
rz_sender plumbing and the 400ms candidate re-send on that leg
- guard is_udp handling against responses to requests that advertised no
udp_port; skip the IPv6 socket bind under force-relay
- test_udp_uat: drop the STUN port race — the punch port must come from the
rendezvous server's TestNatResponse observing this socket's mapping, a
STUN probe from another socket can advertise an unreachable port
- bump hbb_common (webrtc 0.13 MSRV pin rationale + upgrade checklist docs)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- prefer-P2P racing (race_transports_prefer_webrtc) across punch and RelayResponse; ICE bridge with 400ms candidate resend
- controlled-side answerer and ICE routing; sign local DTLS fingerprint into SignedId, controller verifies the binding fail-closed
- fix pc leaks: close_webrtc() on insecure-decline paths (io_loop, port_forward); compute direct before disarming the offerer guard
- point hbb_common to the WebRTC data-plane commit 9f5a296
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Pick up rustdesk-org/rustdesk_desktop_multi_window#37, which re-arms the existing bounded redraw timer whenever a secondary window is shown, including when its first frame was generated while hidden but not presented.
This may perform one delayed child refresh on each show. It intentionally does not add a presentation-complete flag: Flutter reports frame generation rather than successful presentation, so recording success after a synthetic refresh could suppress later self-recovery without a reliable success signal.
