Cancelling the transport that lost the race, and trickle checking before it
holds a pair, are what the design does on every session that connects - and
webrtc-rs reports both at warn, 90 lines of a 386-line controlled-side log,
beside connections that succeeded. agent_internal and peer_connection drop to
error; agent_gather keeps warn, since an unreachable STUN server is the one
upstream signal that explains a session which never connected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M54JAqUK4RynudFou89hod
hbb_common now closes the local-candidate channel when gathering
completes, so the controlled side's forwarder in spawn_webrtc_answerer
ends there, and its signaling connection to hbbs with it, instead of
sitting on a socket hbbs closed at 90s idle for the rest of the session.
It also keeps the reassembly buffer across fragmented frames.
Stream closes the WebRTC peer connection on drop (hbb_common b0b624d),
so the close_webrtc() calls in port_forward and io_loop that sat
immediately before a return or the end of scope did nothing Drop was
not about to do, while the comments beside them still said a bare drop
leaked the pc. Remove both.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019aokqJuhjvB3kijXtAg5Ns
The same link that streams over KCP crawls over WebRTC. webrtc-sctp runs
RFC 4960's AIMD: a fast retransmit halves cwnd, a T3 drops it to one MTU, and
slow start only rebuilds it while data is queued behind it. Where the loss is
random rather than congestion - a lossy long-haul link - the rate settles at
the Mathis ceiling MSS/(RTT*sqrt(p)) however idle the link is: about 1.3 Mbps
at 70ms RTT and 1% loss, 0.6 Mbps at 5%, while 1080p wants 2-5 Mbps. KCP's
turbo profile (nc=1) has no congestion window at all.
The fork now carries a switch that bypasses the two places gating sends on
cwnd, and hbb_common turns it on for every peer connection unless
`allow-webrtc-congestion-control` is set - the same opt-in KCP has in
`allow-kcp-congestion-control`, for the reason at `get_kcp_cc_enabled`.
Sender-side only; a browser or an older build on the other end interoperates.
Measured over a simulated link (35ms one-way, random loss both ways, 12 KB
frames at 30fps, 300 frames): at 1% loss the window stretches 9.9s of video to
20.7s with a mean latency of 5.5s; without it the stream stays realtime at a
mean of 113ms. At 3%: 47s and 15s against 10.2s and 290ms.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019aokqJuhjvB3kijXtAg5Ns
`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
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
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
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
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
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
- 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>
* fix(drm): deliver a rotated output upright instead of sideways (#15886)
the compositor draws a rotated desktop sideways into the landscape
scanout and the physically turned monitor straightens it locally, so the
raw scanout the drm path ships reads sideways in the viewer, and nothing
rebroadcasts on rotation because the framebuffer size never changes.
the capturer now resolves the output transform once per session from the
wayland enumeration, turns accepted frames upright into its own buffer,
and sizes the session in rotated dimensions. the advertised list swaps
width and height for 90/270 outputs, which also makes a mid-session
rotation a topology change that restarts the service, and computes scale
from the post-swap width so a rotated 1:1 monitor no longer advertises
scale 16/9. a non 4-byte format on a rotated session is a hard error and
degrades through the existing health path.
the greeter path where no compositor answers keeps today's behavior:
there is no transform source there. hbb_common carries the new transform
field (submodule bump).
* fix(drm): drop the wayland snapshot when the live layout drifts (#15886)
the advertised list is augmented from the cached wayland snapshot and
nothing invalidated it mid-session, so a rotation the 1.5 s live poll
plainly saw never reached check_changed: the poll reads live, the
advertise kept serving the pre-rotation snapshot. measured before this
commit: transform applied and held, 'desktop layout changed' logged,
zero new encoders. cleared only when the poll saw an actual change, so
the probe cost stays tied to real layout events; after it, the same
stimulus rebuilds into a 1080x1920 encoder within a poll turn.
* refactor: trim comment density to the file norm
* chore: bump hbb_common to the transform field from rustdesk/hbb_common#586
pinned to the #586 commits atop the current pin rather than main tip:
main also carries an unrelated config-keys refactor the app has not
adopted yet, and both #586 commits are reachable upstream through the
merge.
* fix: advertise a lone rotated output at delivered size, one snapshot per session
review findings, both real: the 90/270 swap sat below the origin-only
cut, so a single rotated output advertised unrotated dimensions while
the capturer delivered rotated frames; and transform and origin came
from two get_displays() reads that could straddle a cache invalidation.
the swap now precedes the cut (logical-scale adoption stays multi
output), and new() resolves one snapshot for transform, origin and the
session size, with tests for both. comments trimmed to the three-line
guideline.
* fix(drm): rotate every space the rotation touches, not just the pixels
review findings on #15889, all verified against the code first.
the uinput rect's single-display branches now serve the delivered
orientation, so the pointer reaches the whole of a rotated screen (1).
DisplayRect carries the transform, making 0/180 and 90/270 flips
visible to the drift comparison (5), and the drift poll is an edge on
live-vs-previous rather than a level against the baseline, so the cache
clear fires once per real layout event instead of every 300 ms
forever (9). on the drm path the baseline promotes together with the
clear, so the remap and the client rebase never correct the same origin
delta twice (7), and the poll now runs above the login-screen return,
which was the one place with no other invalidation trigger (6).
a snapshot generation gives a rotation a rebuild path at last (3, 4):
clears bump it, the capturer records it at build, and a stale
generation asks for a rebuild without counting against display health.
the cursor bitmap and hotspot turn with the same session transform the
frames use (11). original_resolution follows the 90/270 swap (12). the
transform comes only from an identity match, never the layout-order
fallback (13), and a missing wayland snapshot at build logs the degrade
instead of silently pinning an unrotated session (10).
unrotate_bgra's body is now libyuv's ARGBRotate, which the existing
direction tests pin to the measured anchor (14). 180 stays master
behavior: i915 advertises hardware rotate-180 and wl_output cannot tell
hardware from software rotation, so undoing it blind would invert an
already-upright frame; it needs the plane rotation property on the
wire (2). the pipewire fallback guard's comment now states the rotated
reality it compares (8).
* fix(drm): one owner for the layout generation, one identity rule for rotation
adversarial pass over the previous commit, three structural findings.
the generation bump rode on the cache clear, which every video-service
start also executes, so any session init or restart tore down every
other live capturer, with no damping against a ping-pong between two
displays. the bump now has a single owner: the edge-detected layout
change in the display-service poll. cache clears are side-effect free
again, and a two-display session survives a third session's init with
zero spurious rebuilds.
the advertise side swapped dimensions for a layout-order-fallback match
while the capturer's transform refused such matches, splitting
advertised size from delivered frames into a black screen. both sides
now key off the same identity-match pass (identity_matches), so a
guessed assignment rotates nothing anywhere.
an edge observed while the drm verdict was transiently non-available
was consumed unpromoted, leaving a rotation sideways for the session;
it now stays owed until the verdict returns. an enumeration that failed
at build pinned transform 0 forever with a warn promising a retry that
did not exist; a missing snapshot now makes the first successful poll
an edge, so the degrade is bounded by the outage. the multi-display
missing-logical-size fallback serves delivered orientation, stale docs
zhou named are updated, and the resolutions list stays mode-space on
purpose: resolution changes ride xrandr, which is inert on this path.
* fix: transpose-tolerant fallback size check, log a rejected rotate geometry
whether a portal stream's caps arrive rotated on a 90/270 output is
unmeasured either way (pipewiresrc does not apply
SPA_META_VideoTransform), and this guard has already broken two readers
who reasoned from its comment - so the size half now accepts either
orientation instead of gambling a permanent offline on one. a source
stride shorter than a row logs the rejected geometry instead of
publishing a silent black frame. comments trimmed to the guideline and
the stale sole-test claim updated.
* fix(wayland): never serve a transposed PipeWire stream
The fallback accepted a stream whose dimensions were the advertised
display's transposed, but CapturerInfo keeps the stream dimensions,
nothing on the wayland side ever reconciles the client afterwards,
and the flutter renderer drops every frame whose size differs from
the advertised display - a permanently blank fallback. Accept only
the exact orientation; a transposed pair now falls into the existing
bail, the display is advertised offline, and the client recovers by
re-enumerating.
* fix(drm): keep the cursor consistent with the session transform
Two holes from the same review pass. The wire cursor id hashes only
the plane pixels and geometry, so a stream rebuilt under a new
transform resent the SAME id and the client's by-id cursor cache kept
the old orientation until the shape itself changed; fold the session
transform into the served id. And a cursor racing new()'s transform
store was processed with transform 0 and never corrected, since the
producer resends only on a shape change; hold that cursor and replay
it once the transform is in - the receive loop wakes at least every
200 ms, so the replay is prompt even on an idle wire.
* fix(wayland): the single-display carve-out must not forgive a transposed stream
The carve-out forgives a size difference (a Full Workspace stream may
report the workspace rather than the mode), but a transposed pair is
the same served-vs-advertised orientation split the previous commit
rejects, and it blanks the client the same way.
* fix(drm): a lone display with a rejected fallback is honestly offline
The transposed rejection promised 'advertised offline', but the
lone-display carve-out in mark_demoted_displays kept the display
online on the grounds that the whole-desktop fallback remains usable
- which is exactly what the rejection just refuted. The video service
then restart-looped against a stream nothing can serve, rebuilding
the portal session about once a second, while the client saw a
display list that lied.
Record the geometry rejection in the display health and let it end
the carve-out; a delivered frame or the demote-cooldown re-arm clears
it, so a recovered output comes back on its own.
* ci: retrigger, the previous run died in the actions outage (all root jobs at exactly 8m)
* fix(drm): a blind capturer owes a rebuild, and name matches reserve globally
Two of the review's findings. A capturer built during a failed wayland
enumeration recorded nothing durable: a later successful enumeration
refills the cache, wayland_snapshot_missing goes false, and the first
live poll sees no edge - the session stays sideways until an unrelated
change. The build now latches that it ran blind and the layout poll
consumes the latch into the existing owed-promotion machinery.
And the identity matcher ran per-connector, so a resolution guess for
an earlier connector could steal a later connector's exact name match
and pin its rotation on the wrong output. Names now reserve in a
global first pass; resolution pairing runs on the remainder only when
forced - one free output and one unmatched connector at that size.
* fix(drm): consume the blind-build latch even on a live-changed poll
Adversarial pass on the previous commit: the short-circuit left the
latch set on exactly the poll where live_changed fired (the common
blind-recovery ordering, since a failed enumeration is not cached and
failed_init makes the first successful poll an edge), and the stale
latch then bought a second, spurious promotion one poll later,
tearing down the freshly rebuilt capturer. The latch is now taken
unconditionally so both edge sources merge into one promotion.
* fix(wayland): hand over a layout change the poll has not seen yet
set_wayland_layout_baseline clears live, which is the edge detector's only
memory of the previous layout. ensure_inited calls it at the top of every video
service start, so a second monitor service starting between a rotation and the
next 1.5s poll recorded the rotated layout as the baseline: the poll then found
baseline == live_rects, owed no promotion, and the first capturer kept its old
transform. Under mutter's software rotation the framebuffer size does not
change and the wayland display-change check is disabled, so the stream stayed
sideways until the next layout event.
The setter now arms the promotion itself when the outgoing live differs from
the incoming baseline, which is the one choke point every caller goes through.
An empty incoming baseline is the DRM-union fallback and proves nothing.
* fix(wayland): the edge detector needs a memory a session init cannot erase
The baseline reset was also the edge detector's memory, so two session inits
straddling a rotation left nothing to compare the next poll against. Keep the
observed layout separate from the per-session input baseline; before the first
poll the outgoing baseline seeds it.
* fix(wayland): a capturer records the layout it was built on
ensure_inited() runs the wayland query before the capturer exists, and a failure
there saves an empty baseline. The capturer's own retry can succeed a moment
later and build on that layout, and because the build was not blind nothing
latched it, so a rotation before the first poll had no memory to be an edge
against and the stream stayed at the old transform.
The build now seeds the edge detector when nothing else has, and only then, so a
capturer built later cannot overwrite what the poll is keeping.
* fix(wayland): keep a capturer record that lost the race with the first poll
The constructor reads its wayland snapshot and records it in the edge
detector in two steps, and the layout poll can land between them. After a
failed session init (empty baseline) the constructor takes layout A and
publishes it, the output rotates, and the poll reads B live: nothing is
recorded yet and the snapshot is present, so it is no edge, and observe()
sets seen=B. The late note_capturer(A) then met a non-empty memory and was
dropped, so the capturer showed A while the detector held B, and B against
B never bumped the generation.
note_capturer now flags a build layout that disagrees with the poll's
memory instead of dropping it (overwriting is still wrong: on a
multi-display session that memory is what the other capturers were built
against). edge() reports the flag as an edge whatever the live layout is,
observe() consumes it right after, and a session init's baseline reset
leaves it alone. Regression test for the interleaving, with the promotion
consuming it, a baseline reset in between, and an agreeing late record as
the control.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011ZwSrP3DFA6ZiPKVHkU5dL
* fix(wayland): a late capturer record from a promoted generation is not a second edge
The record can also land after the poll consumed an edge but before the
bump it promotes, or after the bump with a snapshot taken before it. That
capturer is stale by generation and rebuilds on its own, but the flag it
raised survived the promotion, and the next poll spent a second promotion
on the freshly rebuilt capturers.
Tag the record with the generation the capturer read before taking its
snapshot and count it as an edge only while that generation is current;
the newest generation wins when two records land. Regression test for the
consumed-edge interleaving, with a disagreeing record at the promoted
generation and a stale record after a fresh one as controls.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011ZwSrP3DFA6ZiPKVHkU5dL
* chore: bump hbb_common to main tip
dc95b4f -> 05ed68f, a fast-forward: the flipped-transform warning and the
wlroots xdg-output positions (rustdesk/hbb_common#591, #592), 90-day logs,
the webrtc session cleanup deadlock fix and the hide-general-settings
option. No public API changes and no dependency changes.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011ZwSrP3DFA6ZiPKVHkU5dL
---------
Co-authored-by: rustdesk <71636191+rustdesk@users.noreply.github.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
When Windows is set to "Show only on 1/2", DXGI still enumerates
detached outputs. Preferring that unfiltered list could select a
zero-size display as primary and hang clients waiting for video.
* feat(portable): load per-customer payload from a PE resource
Customizing a Windows client recompiled the packer for every customer,
because data.bin was baked in with include_bytes!. The generic payload is
identical across customers, so only the small per-customer delta needs to
vary: the branded runner exe, custom.txt and the icons.
The packer now also reads an RDPKG RCDATA resource holding a second blob in
the same format, and folds it over the compiled-in payload. A build can then
inject that resource into a prebuilt template instead of running cargo.
The executable to launch comes from the package trailer, and the extraction
directory follows its stem, which replaces the sed of APP_PREFIX. Where the
executable itself is not customized (sciter x86) it stays in the generic
payload and is only renamed, so the merge covers both shapes.
custom.txt keeps being written to disk next to the app: that is what the
client reads at startup and what the updater stages so a customization
survives an upgrade to a stock build.
Also fixes generate.py restoring os.curdir (the literal ".") instead of the
previous working directory, which left it inside the source folder.
CI: ship windows-aarch64 in the unsigned tarball, so ARM custom clients have
a template to build from.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q7fBdTwziR5BHTkSz7Tzcm
* ci: publish msi templates for custom client builds
Custom clients rebuild the msi through WiX for every customer, though the
package only differs by the app name, a few GUIDs and four files.
Build the msi once more per release with a __RDAPPNAME__ placeholder and ship
it unsigned in the unsigned tarball, so a customer's build can patch it rather
than run msbuild. It stays unsigned because patching would invalidate a
signature anyway.
Doing this in CI is what makes ARM custom clients possible: preprocess.py runs
the packaged exe to read its version and build date, so an arm64 msi can only
be produced on a native arm64 machine, which the runner already is and the
build agents are not. Patching runs no exe, so an x64 agent can then patch the
arm64 template.
preprocess.py rewrites res/msi in place and locates the app as <app-name>.exe
inside the dist, so the tree is reset around the second build and the dist copy
is renamed to match. Sciter x86 ships no msi and is untouched.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q7fBdTwziR5BHTkSz7Tzcm
* refactor(msi): pass the app name to the printer custom actions
preprocess.py rewrote the CustomActions sources per customer so the printer
carried the app name, which meant the dll was recompiled for every custom
client and, worse, left the app name baked into a compiled binary.
Pass it through CustomActionData instead. Only the printer and its port ever
varied: the INF path and the driver name ship under their stock names and
preprocess.py already forced the driver name back to RustDesk, so a single
build of the dll now serves every custom client.
Both actions treat the name as optional and fall back to the stock name, so a
package built before this still installs and uninstalls its printer.
This also unblocks patching a prebuilt msi template, which cannot work while a
compiled dll contains the app name: replacing a string inside a PE would shift
everything after it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q7fBdTwziR5BHTkSz7Tzcm
* ci: use an 8.3-safe placeholder for the msi template
WiX derives a short name for any name that is not valid 8.3, and a patch
cannot rewrite a truncated placeholder, so a long placeholder would leave the
package's short names pointing at it. RDAPPNAM is eight characters like
"RustDesk" and needs no short name, keeping the template as close to the
shipped package as the mechanism allows.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q7fBdTwziR5BHTkSz7Tzcm
* feat(msi): give a template its own cabinet for per-customer files
Rebranding recompressed the whole ~100MB payload because one cabinet held
everything. In template mode preprocess.py puts the handful of files a custom
client replaces on a second cabinet, so a patch rebuilds a few hundred KB and
leaves the payload cabinet alone. The shipped msi is built without template
mode and keeps its single cabinet.
The branding assets need conditional components. A stock build ships none of
them -- there is no icon.ico, icon.png or logo*.png, only icon.svg -- so the
template has to carry placeholders for the File rows to exist, and a customer
supplies whichever they want. Installing a placeholder unconditionally would
give a customer with no logo a placeholder image, where today a missing asset
means no logo at all: the client tries each candidate and treats the failure as
absence. So each optional asset installs only when its property says the
customer supplied one.
CI creates those placeholders and builds the template with the new mode.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q7fBdTwziR5BHTkSz7Tzcm
* ci: build the msi template with a sentinel revision
preprocess.py appends a build-time revision as the fourth version field, so a
template built without one would bake the CI clock into every customer's
package. Revision 0 marks the field as the patcher's to fill in, and makes the
template deterministic.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q7fBdTwziR5BHTkSz7Tzcm
* fix(portable): delete files a later package no longer carries
The extraction directory is wiped only when the packer's compiled-in timestamp
changes. That used to be per customer, because generate.py ran for each build;
now the packer is compiled once per release, so every customer and every
rebuild within a release share one timestamp and nothing is ever wiped.
A customer who removes their logo and rebuilds would therefore keep showing it:
the new package simply omits logo.png, and md5 skipping only covers files that
are still present. Record the package's paths in the extraction's meta file and
delete the ones a later package drops.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q7fBdTwziR5BHTkSz7Tzcm
* fix(portable): build the dropped-file path from plain components
meta.toml lives in a user-writable directory and now drives deletion, but the
traversal guard tested the normalised string while the join used the raw one.
Path::join replaces the base outright when handed an absolute path, so an
edited meta.toml could point remove_file anywhere.
The path is now rebuilt from Normal components only. A colon is rejected
explicitly rather than left to the host's parser: a drive-relative "C:x" parses
as a Normal component everywhere, and only a Windows host reads "C:/..." as a
prefix, so the same input escaped when the logic was exercised off-Windows --
which is what the new test catches.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q7fBdTwziR5BHTkSz7Tzcm
* fix(msi): pass the printer name in a format the custom action can read
[~] is MSI's escape for a NUL character, not the delimiter WcaReadStringFromCaData
splits on -- that is a literal wide char 128, which a Formatted property value
cannot carry -- and WcaGetProperty returns a null-terminated string anyway. So
the second field was unreachable: InstallPrinter always fell back to the stock
name and installed a printer and port called "RustDesk Printer" inside a
customer's branded package, while UninstallPrinter, whose data is a single field
and parsed fine, went looking for "Acme Printer" and left the real one behind
for good.
Both actions now read CustomActionData directly and split on a character that
cannot occur in a Windows path or in a validated app name. A package built
before this carries no separator and keeps the stock name, as it did.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q7fBdTwziR5BHTkSz7Tzcm
* fix(portable): retry failed stale branding cleanup
Signed-off-by: fufesou <linlong1266@gmail.com>
* fix(portable): reject malformed RDPKG resources
Distinguish an absent customer package from an invalid resource and
propagate package errors instead of launching the stock payload.
Signed-off-by: fufesou <linlong1266@gmail.com>
* refact: format 2 files
Signed-off-by: fufesou <linlong1266@gmail.com>
* fix(msi): match process names case-insensitively during uninstall
Signed-off-by: fufesou <linlong1266@gmail.com>
* fix(custom-client): validate portable exclusion and MSI action data
Fail when --exclude-exe does not match a file, and propagate MSI
CustomActionData read failures while preserving legacy fallback behavior.
Signed-off-by: fufesou <linlong1266@gmail.com>
* fix: generate.py, exclude-exe
Signed-off-by: fufesou <linlong1266@gmail.com>
* Revert "fix: generate.py, exclude-exe"
This reverts commit 5104664e95.
* fix: simple path fix in generate.py
Signed-off-by: fufesou <linlong1266@gmail.com>
* Remove useless comments
Signed-off-by: fufesou <linlong1266@gmail.com>
* fix(portable): remove expect() anyway
Signed-off-by: fufesou <linlong1266@gmail.com>
* fix(portable): validate executable path boundaries
Reject executables outside the source folder and
reuse the package path normalization logic during
stale file cleanup.
Signed-off-by: fufesou <linlong1266@gmail.com>
* fix, remove useless file
Signed-off-by: fufesou <linlong1266@gmail.com>
---------
Signed-off-by: fufesou <linlong1266@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: fufesou <linlong1266@gmail.com>
* fix(wayland): back off the polling display lookups after a failure (drm)
In drm builds an enumeration that fails with no endpoint named in the
environment falls back to the socket probe, which forks a child bounded by
seconds, and the display service asks again every 300 ms -- at a greeter
with no reachable compositor that is a probe child per turn, forever. Such
a failure now stamps a shared 5 s backoff, and only the polling callers
honor it: the 300 ms displays-changed check skips its turn and the 1.5 s
live layout poll returns no answer for that turn.
Only the failure that would fork stamps. A session server is spawned with
WAYLAND_DISPLAY set, so its failed connect bails in-process before any
fork; stamping there would buy nothing and cost recovery latency, so live
sessions keep master's behavior exactly. The stamp also survives
clear_wayland_displays_cache: it describes the seat, not the cache, and
the ~1/s capturer rebuild loop clears on every teardown -- dropping the
stamp with the cache would let that loop defeat the backoff and would
turn every post-hotplug failure into a "first" one forever.
The displays-changed check weighs the backoff against what is already
published. With nothing synced yet it always populates -- an unaugmented
DRM list beats the empty broadcast the send path would otherwise emit.
With a synced layout, a suppressed turn keeps it, and a fresh first
failure keeps it too; only a failure that persists across a backoff
replaces it with the DRM stack, so a hotplug at a failing seat converges
within one backoff while a transient failure never tears down a good
layout.
One-shot callers -- session init, pipewire stream setup, capturer info --
keep probing fresh through get_displays, whose failure semantics are
unchanged: replaying a transient failure there would latch an empty answer
into session-long state. Non-drm builds compile none of this.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(wayland): log DRM lookup failure once
* fix(wayland): reset lookup warning after recovery
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix(linux): serve the Wayland login screen the DRM backend was built for
The login screen support in #15420 never worked on a real greeter. fufesou found
it: the session is refused, and with the refusal commented out the client gets a
failed connection instead of a screen.
One premise under all of it. `get_values_of_seat0` is
`_get_values_of_seat0(.., ignore_gdm_wayland = true)`, so a gdm/sddm Wayland
session is skipped by construction and `get_display_server` falls back to x11.
That was correct while the portal was the only backend, since the portal cannot
serve a greeter at all. The DRM path never talks to the compositor, which is
precisely why it can serve one, so the premise stops holding there and every
x11-vs-Wayland decision in the tree answers x11 at a login screen.
The central change is the memoised `IS_X11`: when it reads x11 and seat0 is a
Wayland greeter, answer Wayland. That covers fifteen routing sites at once, and
it is under `cfg(feature = "drm")`, so a build without the backend keeps the
current answer exactly. `is_x11_for_drm` is the unmemoised form for the two
retry loops that must keep asking while a boot is still naming the session, and
the memoised accessor is scoped to per-frame callers in the per-session
`--server`, which the service only spawns once it has identified the session.
Input was the last layer and lived outside all of that. `Enigo` decides
x11-vs-Wayland once in `Default::default()`, from the same seat0 lookup, and on
"x11" routes every key and mouse event to xdo; with no X server that context is
null and libxdo drops them without an error. So the uinput devices were created,
the compositor opened them, and nothing was ever written to them. `set_is_x11`
is now called where the custom devices are installed, which is only reached once
`!is_x11()` is already established. The unit test pins both directions, since a
one-directional test passes against the bug.
With no compositor reachable, the uinput desktop rect comes from the DRM display
list instead: those are the same displays being captured, so the coordinate space
matches by construction. Telling the truth about a greeter also makes four
compositor-probing paths reachable where the probe cannot answer; all four
already treat an empty output list as "nothing to do", so they skip it and 11818
"Could not find wayland compositor" warnings in one session became 1.
Tested on an sddm Plasma Wayland greeter, MacBook T2, 2880x1800: the greeter
renders, typing from the client enters characters in the password field, a click
at an absolute coordinate opens the greeter session combo, the service pre-warm
primes in 994 us instead of timing out, and the privileged service maps no EGL
during a live capture. Not proven on gdm under Wayland.
Known limitations: non-ASCII characters cannot be typed at a greeter, because
that path goes through the clipboard and the clipboard here is X11 only; and at
a multi-monitor greeter the pointer reaches the first display only, since every
DRM output reports origin (0,0) on Wayland and there is no arrangement to derive
without the compositor.
* fix(linux): a Wayland greeter the DRM backend can serve is not headless
fufesou reported the login screen still failing on Ubuntu 24.04 with gdm3, with
the client asking for OS credentials to start an X session instead of showing the
greeter. Reproduced on a real gdm greeter here.
Same premise as the rest of the branch, one more consumer. `DesktopManager::new`
reads seat0 through `get_values_of_seat0`, which skips a gdm/sddm Wayland session
by construction, so at a greeter it finds no session at all and
`get_supported_display_seat0_username` returns None from its empty-username arm.
That makes `is_headless()` true, so the service advertises headless and
`try_start_desktop` answers `LOGIN_MSG_DESKTOP_SESSION_NOT_READY`. The corrected
`IS_X11` does not reach this one: it asks who owns seat0, not which display
server is running.
So ask again, with the greeter visible, when the DRM backend can capture and
inject into it. At query time rather than in `new()`, because the DRM probe has
not necessarily settled when the desktop manager is constructed, and the answer
would latch for the process lifetime. In a normal session the latched username is
a real user and the extra read is skipped.
* chore: drop the hbb_common bump, this branch does not need it
The bump carried rustdesk/hbb_common#580, the compositor-socket fallback. Nothing
here depends on it: the greeter paths in this branch are the ones that run when
compositor data is unavailable, which is what the commit before this one states as
a known limitation. Keeping the bump would only block the greeter fix behind a
review of a separate change, and would import that change's blocking review items
into this path.
* fix(linux): let the uinput uid gate see the greeter that owns seat0
Input at a real greeter was rejected by our own authorization. Measured on Ubuntu
24.04 with gdm3: the root service logs
Rejected unauthorized connection on uinput ipc channel:
postfix=_uinput_control, peer_uid=Some(120), active_uid=None
and the greeter's `--server` gets ECONNRESET out of `setup_uinput`, so no uinput
device is ever created and neither keyboard nor mouse reaches the greeter.
uid 120 is gdm, the owner of the only active seat0 session. `active_uid` is None
because the uinput authorizer deliberately bypasses the service-loop cache and
takes a fresh seat0 lookup, and the fresh read hides a Wayland greeter by
construction. The cache-based gates do not have the problem: `Desktop::refresh`
fills it through the greeter-visible read, which is also why capture and config
sync work at a greeter while input does not.
So make the fresh read agree with the cache. It keeps the property the uinput gate
wants, a lookup that cannot be stale, and it still compares the peer against the
uid of the session that owns seat0 -- which at a greeter is the greeter.
* fix: settle the DRM probe before routing login to X11, and read seat0 fresh
Two findings from the #15792 review, both verified against the code:
- drm_login_screen_seat0_username asked the cached probe, so a client
arriving before warm_availability publishes its verdict read "no DRM"
and, with allow-linux-headless=Y, try_start_x_session could start Xorg
over a live Wayland greeter. Ask the probing form instead, and only
after the cheap seat0 read says a Wayland greeter is actually there: a
bounded definitive verdict is affordable on a login-time path.
- get_supported_display_seat0_username trusted the seat0 values cached in
DesktopManager::new(), which go stale across a logout or a fast user
switch: a stale non-greeter name skipped the greeter probe and was
returned as the supported display owner. Read seat0 fresh on every
query; every call site is connection-time, so the extra loginctl read
is cheap.
Regression-tested on a real sddm Wayland greeter: capture streams the
greeter, the RustDesk password dialog is the only prompt, and five typed
characters appeared in the greeter password field over uinput with zero
"Rejected unauthorized connection" lines in the service log.
* fix: ask the greeter compositor for the multi-monitor layout
The display arrangement and the pointer mapping were wrong at a
multi-monitor login screen, and the mechanism is measured on a two-head
virtio VM: DRM has no origins, so every display was advertised at (0,0)
(a stacked arrangement on the client), and the uinput range was taken
from the union of the DRM modes while the compositor had arranged the
outputs side by side.
Both came from the same premise, written before the hbb_common socket
fallback existed: "a login screen has no compositor to ask".
wayland_outputs_askable() skipped the wl_output augmentation at any
greeter, and update_uinput_resolution took the DRM union directly. The
premise is false now: a greeter runs a compositor, and the socket
fallback reaches it with no environment variables, measured answering
two outputs at the VM greeter while the old gate was still routing
around it.
Drop the gate and take the compositor-first path everywhere. Where the
fallback cannot answer, the output list comes back empty and both call
sites degrade to exactly the old behavior, so a build against an older
hbb_common is unchanged.
* fix: augment a single display too, and probe the desktop rect off the executor
Two follow-ups from the automated re-review of cd80c3dee, both verified:
- augment_with_wayland_geometry skipped the compositor below two DRM
displays, but on a multi-GPU host the one connector this service can
open may sit at a non-zero origin of the compositor layout, and DRM
alone reports (0,0).
- the desktop rect for uinput can now block for the socket probe
deadline, and update_uinput_resolution runs on current-thread
runtimes; move the query into spawn_blocking.
The third re-review finding, the warm-up allegedly skipping Wayland
greeters, is refuted: warm_availability probes while is_x11_for_drm()
is false, which includes a Wayland greeter, and the greeter log of the
VM run behind cd80c3dee shows the warm succeeding there.
* fix: baseline the layout from the blocking task, and augment a lone output's origin
The layout snapshot after the rect lookup still ran on the executor: a
failed compositor lookup is not cached, so the snapshot synchronously
repeated the whole socket probe there. The baseline is now computed
inside the same blocking task, from the snapshot the successful lookup
just cached, or omitted when only the raw DRM union was available,
which keeps the #15601 remap inactive exactly where origins are
unknown.
A single compositor output now hands its origin to a single connector:
the lone output can sit at a non-zero origin the DRM side cannot see.
Scale stays 1 on purpose, matching how a single display is advertised
at physical size, and more connectors than the one output stays
unaugmented, since the layout-order fallback would plant that origin on
a guess.
Also refresh the get_primary_index doc that still said augmentation
declines below two connectors.
* fix: read the DRM probe as a tri-state, and keep pre-auth seat0 checks cache-only
is_available() answered false both for a definitive no-DRM verdict and
for a probe that had simply not settled (another probe in flight, or a
failure still below the disable threshold), and the login-screen
decision turned that transient false into no-greeter: try_start_x_session
could put Xorg over a live greeter in exactly the window the probe
needed. The machinery now answers Available/Unavailable/Unsettled, and
only a definitive Unavailable routes the seat toward X11.
Connection setup also ran the whole lookup pre-auth: constructing
LinuxHeadlessHandle called is_headless() before authentication, holding
DESKTOP_MANAGER while loginctl ran and, at a greeter, while the DRM
probe waited out its handshake. An unauthenticated peer could occupy a
worker for seconds and serialize every other connection on the mutex.
is_headless() now answers from a snapshot refreshed off-thread, and the
fresh lookup became a free function called with the manager lock
released everywhere; the enforcing decisions, get_username and
try_start_x_session, still read seat0 fresh.
Also drops seat0_display_server, dead since the fresh-read change.
* fix: respect RUSTDESK_FORCED_DISPLAY_SERVER over the greeter correction
The greeter correction rewired IS_X11 and is_x11_for_drm() to Wayland
whenever seat0 looks like a Wayland greeter, including when the operator
explicitly forced the display server: get_display_server() kept honoring
the override while the DRM routing gates contradicted it, leaving
capture and input routing internally inconsistent. The correction now
only adjusts the auto-detected answer.
* fix: honest pre-auth snapshot, sticky negative verdict, and a complete forced-x11 gate
Four defects found by an adversarial review of the two previous
commits, all in their new lines:
- The empty-snapshot fallback derived headless from the manager's
boot-time seat0 read, which is blank at a Wayland greeter (the
loginctl wrapper skips greeter sessions), so the first connection of
every server process at a greeter answered headless=true, the
opposite of the comment on it. No snapshot now answers NOT headless,
the snapshot is seeded at start_xdesktop, and the boot-time cache is
gone entirely (it had no reader left).
- wait_desktop_cm_ready gated on a bool stored at construction, which
can lag one seat0 transition behind and skipped the CM-ready wait
right after a logout. It re-reads the snapshot at call time.
- A settled Unavailable was erased at NEGATIVE_TTL expiry (state to
Unknown, failure counter to zero), so a permanently helper-less box
reopened the Unsettled window every 30 seconds and the login decision
kept adopting a greeter nothing can serve. The verdict now stays
Unavailable while an off-thread re-probe re-verifies it: a failed or
empty re-probe restamps the no, and only a non-empty list flips it.
- The forced-x11 gate only covered IS_X11 and is_x11_for_drm, while
the seat0 adoption path still probed DRM and admitted greeter
sessions whose capture and input then routed to X11. Greeter
adoption now yields to an operator-forced X11, degrading to upstream
behavior: the connection is refused at the login screen.
* fix: keep the login request path off the probe entirely
try_start_desktop runs while handling a LoginRequest, before password
validation, and at a Wayland greeter its seat0 lookup reached the
probing availability form: an unauthenticated peer could park a worker
for the probe deadline. The greeter adoption now reads a cached
tri-state that never blocks; when the state is Unknown it kicks the
probe off-thread and answers Unsettled, which the login decision treats
as a possibly servable greeter until it settles. Settling lives in the
startup warm-up, that kick, and the TTL re-verifiers; the blocking form
stays for the capture-side callers, where waiting is acceptable.
* fix: run the pre-auth desktop start off the executor, guard the refresh flag, trim comments
From fufesou's #15792 re-review (no blocking issues) plus a bot pass:
- try_start_desktop now runs on spawn_blocking. It executes loginctl,
and PAM when a session must start, while handling a LoginRequest
before password validation, so a slow logind must not tie up an async
request worker; the blocking pool absorbs it.
- kick_seat0_refresh releases SEAT0_REFRESH_IN_FLIGHT through an RAII
guard, so a panic in the refresh thread cannot freeze is_headless on a
stale snapshot for the process lifetime.
- drm_can_serve_login_screen stays Available-only, and the reason is now
in the code: it is deliberately not symmetric with the seat0 adoption
gate. Adoption yields Xorg only on a definitive Unavailable; admission
accepts only on a definitive Available; both wait through an unsettled
probe. Admitting there would black-screen a client on a helper-less
box, so a review suggestion to make them agree is declined.
- Trimmed two over-long comments to the repo's three-line rule.
* fix(linux): harden DRM login-screen startup
Keep unauthenticated headless checks cache-only, bound OS-session startup to one blocking task, and surface JoinError failures.
Wire the isolated Wayland probe consumer and update hbb_common plus libdrmtap 0.5.4.
* fix(linux): headless refresh state
Signed-off-by: fufesou <linlong1266@gmail.com>
* fix(linux): keep headless startup state consistent
- gate concurrent desktop startup attempts
- route CM IPC after refreshing desktop state
- avoid blocking seat0 queries in the CM retry loop
- preserve newer seat0 snapshots during overlapping refreshes
- derive DRM geometry and primary display from one Wayland snapshot
Signed-off-by: fufesou <linlong1266@gmail.com>
---------
Signed-off-by: fufesou <linlong1266@gmail.com>
Co-authored-by: rustdesk <71636191+rustdesk@users.noreply.github.com>
Co-authored-by: rustdesk <info@rustdesk.com>
Co-authored-by: fufesou <linlong1266@gmail.com>
`try_xrandr_primary` runs a bare `Command::new("xrandr").output()`. Its two
siblings in the same file, `try_kscreen_primary` and the gdbus one, both go through
`run_with_timeout(.., COMMAND_TIMEOUT)`, and the comment above that helper says why:
these commands are known to hang. xrandr is the one left bare.
It matters because of where it runs. `get_primary_monitor` is called from
`get_displays` with the process-wide `DISPLAYS` guard held, and on a Wayland host
the caller can be the service, which has no DISPLAY and no session bus. An X client
that blocks there blocks every consumer of the display list behind the same lock.
No behaviour change when xrandr answers: same command, same parsing, one second of
patience.
