Skip to content

feat(server): expose session-lifetime baseline RTT to the embedder - #1737

Merged
Marc-André Moreau (mamoreau-devolutions) merged 1 commit into
Devolutions:masterfrom
lamco-admin:feat/server-baseline-rtt-handle
Aug 21, 2026
Merged

Marc-André Moreau (mamoreau-devolutions) merged 1 commit into
Devolutions:masterfrom
lamco-admin:feat/server-baseline-rtt-handle

Conversation

@glamberson

Copy link
Copy Markdown
Contributor

Summary

  • RttSnapshot.min_ms is a sliding-window low that can rise as low samples
    age out of the window, and is explicitly documented as not baseRTT per
    MS-RDPBCGR 2.2.14.1.5, which defines baseRTT as the session-lifetime
    lowest. AutoDetectManager already tracks that true low internally as
    min_rtt_ms for the wire NetworkCharacteristicsResult, but nothing
    exposed it as its own value: an embedder reading snapshot() or
    autodetect_rtt_handle() alone cannot derive averageRTT - baseRTT as a
    queueing-delay signal, since that relationship only holds when the
    floor cannot rise.
  • Adds AutoDetectManager::baseline_rtt_ms() as the public getter for the
    existing field, and mirrors the autodetect_rtt_handle plumbing on
    RdpServer: a new autodetect_baseline_rtt: Arc field,
    autodetect_baseline_rtt_handle() accessor, and
    with_autodetect_baseline_rtt_handle() builder method.
  • A matched RTT sample always updates min_rtt_ms in the same
    handle_response call that returns it, so the store site reads the new
    getter unconditionally right after storing the RTT sample, with no
    before/after comparison needed (unlike the bandwidth case in feat(server): expose measured bandwidth to the embedder #1734,
    where the underlying value can be cleared to None on an unusable
    measurement).
  • Adds a manager-level test pinning the session-low-not-window-low
    property on the new getter directly, plus the usual pair of
    handle-plumbing tests (sentinel default, injected-handle round trip).

Validation

cargo xtask check fmt/lints/tests/typos/locks all pass, including the
3 new tests.

Notes

No public API break: RdpServer::new is crate-private, and the public
surface (RdpServerBuilder) only gains an additive optional field and a
new method.

RttSnapshot.min_ms is a sliding-window low that can rise as low samples
age out of the window, and is explicitly documented as not baseRTT per
MS-RDPBCGR 2.2.14.1.5, which defines baseRTT as the session-lifetime
lowest. AutoDetectManager already tracks that true low internally as
min_rtt_ms for the wire NetworkCharacteristicsResult, but nothing
exposed it as its own value: an embedder reading snapshot() or
autodetect_rtt_handle() alone cannot derive averageRTT - baseRTT as a
queueing-delay signal, since that relationship only holds when the
floor cannot rise.

Add AutoDetectManager::baseline_rtt_ms() as the public getter for the
existing field, and mirror the autodetect_rtt_handle plumbing on
RdpServer: a new autodetect_baseline_rtt: Arc<AtomicU32> field,
autodetect_baseline_rtt_handle() accessor, and
with_autodetect_baseline_rtt_handle() builder method. A matched RTT
sample always updates min_rtt_ms in the same handle_response call that
returns it, so the store site reads the new getter unconditionally
right after storing the RTT sample, with no before/after comparison
needed.

Adds a manager-level test pinning the session-low-not-window-low
property on the new getter directly, plus the usual pair of
handle-plumbing tests (sentinel default, injected-handle round trip).
@github-actions github-actions Bot added kind/protocol Affects RDP or related protocol behavior needs-review A human reviewer is the current next actor risk/medium Behavioral change that does not substantially alter a core public API size/S Size: up to 199 counted lines and 5 files; exceeds XS in either measure labels Aug 21, 2026
@mamoreau-devolutions
Marc-André Moreau (mamoreau-devolutions) merged commit ac7800c into Devolutions:master Aug 21, 2026
44 checks passed
Anton Mostovoy (antonmos) added a commit to antonmos/IronRDP that referenced this pull request Aug 24, 2026
…e public API)

Six upstream commits since the last sync, in crates/ironrdp-server:
Devolutions#1242 (typed ServerError, breaking), Devolutions#1417 (rdpeusb server integration),
Devolutions#1773 (RDPEI wiring), Devolutions#1737 (RTT baseline), Devolutions#1769 (mouse position, no
server.rs footprint), Devolutions#1754 (test relocation, no server.rs footprint).

Only one real conflict, in run()'s loop -- the same class as prior
syncs: HEAD's preemption race vs upstream's plain accept-loop tail.
Resolved by taking HEAD's side (a strict superset).

The change that needed real attention, not just conflict resolution:
Devolutions#1242 makes run/run_connection/run_connection_with return ServerResult
(a new typed error), keeping the anyhow-returning bodies as private
_inner methods -- exactly the same wrap-and-rename shape Devolutions#1721 already
used for run_connection_with (now a third layer: run_connection_with ->
run_connection_with_inner [Devolutions#1242, does the Devolutions#1721 static-channels reset]
-> run_connection_inner [the real body, PendingConnection-based here]).
Upstream's own accept loop switched to calling run_connection_with_inner
directly instead of the now-ServerResult-returning public wrapper, since
its `conn`-equivalent needs anyhow::Error for on_disconnected and a
consistent type across match arms.

This branch's preemption race has the identical two call sites
(Entry::Fresh's stream, in both the preempt-enabled race and the
plain-queue-behind fallback) that were still calling the public
run_connection -- which would have silently started returning the wrong
error type (ServerResult vs serve_negotiated's anyhow Result, breaking
the match arms' common type) had the merge not been caught by an actual
build. Switched both to run_connection_with_inner(stream,
TransportTls::Managed), matching upstream's own fix exactly, with a
comment explaining why (this is not a stylistic change -- it's required
for the code to typecheck at all once the two arms feeding into `conn`
must share one Result type).

Devolutions#1417 (USB) and Devolutions#1773 (RDPEI)'s server.rs footprint is additive and,
for USB, feature-gated (`#[cfg(feature = "usb")]`) so it's a no-op in
every build this branch's own verification exercises; Devolutions#1754 relocated
the autodetect unit tests out of this crate into ironrdp-testsuite-core,
so ironrdp-server's own suite drops from 24 to 14 tests -- confirmed by
grep that they still exist there, not lost.

14/14 tests pass in both default and --features egfx (down from 24,
accounted for above, not a regression), clippy --all-targets clean in
both, fmt clean, full cargo build --workspace green.
@glamberson
Greg Lamberson (glamberson) deleted the feat/server-baseline-rtt-handle branch August 30, 2026 15:12

This branch was previously deployed

1 inactive deployment
llm-providers — 0117e083 Deployed Aug 21, 2026 by glamberson via Classify pull request #3146
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/protocol Affects RDP or related protocol behavior needs-review A human reviewer is the current next actor risk/medium Behavioral change that does not substantially alter a core public API size/S Size: up to 199 counted lines and 5 files; exceeds XS in either measure

Development

Successfully merging this pull request may close these issues.

2 participants