fix: don't let the preferred branch's own relay preempt a direct fallback

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
This commit is contained in:
rustdesk
2026-08-06 08:11:22 +08:00
parent 2f8c8704bb
commit c631cdcf22

View File

@@ -211,13 +211,16 @@ impl Drop for OffererGuard {
///
/// A plain `select_ok` would almost always pick the relay: its TCP connect completes in one
/// round trip while WebRTC needs candidate trickle + ICE checks + DTLS + SCTP. Instead:
/// - a WebRTC success is committed immediately;
/// - an `is_p2p` result from `others` (e.g. IPv6 direct) is committed immediately;
/// - a relayed result inside the preference window is *held*, giving WebRTC until the window
/// expires to connect and take priority. The window starts when the first relay is ready, not
/// when setup begins, so rendezvous latency cannot consume the preference budget. Unfinished
/// `others` keep racing so a slower direct transport can still win; when the window ends (or
/// WebRTC fails) the held connection is committed;
/// - an `is_p2p` result from either side (WebRTC, or e.g. IPv6 direct from `others`) is committed
/// immediately;
/// - a relayed result from *either* side inside the preference window is *held*, giving the other
/// side until the window expires to land something direct. `webrtc_fut` is a whole punch
/// attempt, not just the WebRTC connect, so it too can end in a relay, and committing that
/// immediately would preempt a direct punch still in flight on the other branch. The window
/// starts when the first relay is ready, not when setup begins, so rendezvous latency cannot
/// consume the preference budget. Unfinished `others` keep racing so a slower direct transport
/// can still win; when the window ends (or the other side fails) the held connection is
/// committed;
/// - a failure on one side commits the survivor as soon as it succeeds; two failures compose
/// into one error.
///
@@ -246,8 +249,22 @@ async fn race_transports_prefer_webrtc<'a, T: 'a>(
}, if webrtc_fut.is_some() => {
webrtc_fut = None;
match res {
// WebRTC connected: preferred outright; a held relay conn just drops.
Ok(conn) => return Ok(conn),
// A direct connection is the outcome this race exists to protect: commit it
// outright, and a held relay conn just drops.
Ok(conn) if is_p2p(&conn) || others_fut.is_none() => return Ok(conn),
// `webrtc_fut` is a whole punch attempt, not just the WebRTC connect, so it
// can end in a relay of its own. Committing that immediately would preempt a
// direct punch still in flight on the other branch — the exact inversion this
// function exists to prevent — so hold it on the same terms as any relay.
Ok(conn) => {
if held.is_none() {
held = Some(conn);
window.as_mut().reset(
Instant::now() + Duration::from_millis(window_ms),
);
window_started = true;
}
}
Err(e) => {
if let Some(conn) = held.take() {
return Ok(conn);
@@ -287,7 +304,14 @@ async fn race_transports_prefer_webrtc<'a, T: 'a>(
}
Err(e) => match webrtc_err.take() {
Some(we) => bail!("WebRTC failed: {}; fallback failed: {}", we, e),
None if webrtc_fut.is_none() => return Err(e),
None if webrtc_fut.is_none() => {
// The preferred branch may have parked a relay here; nothing else can
// win now, so commit it rather than failing the connection.
match held.take() {
Some(conn) => return Ok(conn),
None => return Err(e),
}
}
None => others_err = Some(e),
},
}
@@ -5202,6 +5226,48 @@ mod webrtc_race_tests {
assert_eq!(got, "webrtc");
}
// The preferred branch runs a whole punch attempt, so it can itself end in a relay. That must
// not preempt a direct punch still in flight on the fallback branch.
#[tokio::test]
async fn relay_from_preferred_branch_does_not_preempt_a_direct_fallback() {
let got = race_transports_prefer_webrtc(
ok_after(10, "preferred-relay"),
vec![ok_after(120, "direct")],
60_000,
|tag| *tag == "direct",
)
.await
.unwrap();
assert_eq!(got, "direct");
}
// ...but it is still committed once nothing direct can arrive.
#[tokio::test]
async fn relay_from_preferred_branch_committed_when_fallback_fails() {
let got = race_transports_prefer_webrtc(
ok_after(10, "preferred-relay"),
vec![err_after(50, "punch failed")],
60_000,
NOT_P2P,
)
.await
.unwrap();
assert_eq!(got, "preferred-relay");
}
#[tokio::test]
async fn relay_from_preferred_branch_committed_when_window_expires() {
let got = race_transports_prefer_webrtc(
ok_after(10, "preferred-relay"),
vec![ok_after(60_000, "too-slow")],
100,
NOT_P2P,
)
.await
.unwrap();
assert_eq!(got, "preferred-relay");
}
#[tokio::test]
async fn preference_window_starts_when_relay_is_ready() {
let got = race_transports_prefer_webrtc(