Skip to content

feat(server): send periodic Heartbeat PDUs on the message channel - #1842

Merged
Marc-André Moreau (mamoreau-devolutions) merged 1 commit into
Devolutions:masterfrom
lamco-admin:feat/server-heartbeat-emission
Aug 30, 2026
Merged

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

Conversation

@glamberson

Copy link
Copy Markdown
Contributor

The server never sends Server Heartbeat PDUs (MS-RDPBCGR 2.2.16.1), even though the client half of the feature is in place: HeartbeatPdu encode/decode landed with #1814 and clients decode and ignore them. This adds the send half, opt-in.

RdpServer::enable_heartbeat(HeartbeatConfig) arms a fifth joined future in the client loop, following refresh_auto_reconnect_cookie's shape. It only runs when three gates hold: the feature is enabled, the client requested a message channel, and the client advertised RNS_UD_CS_SUPPORT_HEARTBEAT_PDU in its GCC early capability flags. That last gate needed the acceptor to expose those flags, so AcceptorResult gains client_early_capability_flags, populated the same way #1453 exposed multitransport_flags.

Two spec details drove the shape. Heartbeats ride the MCS message channel framed by a Basic Security Header carrying SEC_HEARTBEAT, exactly like auto-detect after #1348, so encode_heartbeat mirrors encode_autodetect_request. And 2.2.16.1 says heartbeats SHOULD only be sent when no other PDU went out in the interval, so SharedWriter now counts successful writes and the heartbeat tick skips the send when the count moved: ordinary traffic doubles as the liveness signal.

The defaults (period 5 seconds, warning after 3 missed heartbeats, reconnect after 5 more) are this library's own choice; the spec mandates no values and the client MAY ignore both counts. A zero period is bumped to one second to keep the timer well-formed.

No new tests: the wire format is already covered by the HeartbeatPdu round-trip tests #1814 added in ironrdp-pdu, and ironrdp-server builds no test harness for the client-loop plumbing this touches.

cargo xtask check fmt/lints/tests/typos/locks all pass.

The server never sends Server Heartbeat PDUs (MS-RDPBCGR 2.2.16.1), even
though the client half of the feature is in place: HeartbeatPdu
encode/decode landed with Devolutions#1814 and clients decode and ignore them. Add
the send half, opt-in via RdpServer::enable_heartbeat(HeartbeatConfig).

A fifth joined future in the client loop follows the shape of
refresh_auto_reconnect_cookie. It runs only when three gates hold: the
feature is enabled, the client requested a message channel, and the
client advertised RNS_UD_CS_SUPPORT_HEARTBEAT_PDU in its GCC early
capability flags. That last gate needs the acceptor to expose those
flags, so AcceptorResult gains client_early_capability_flags, populated
the same way multitransport_flags was in Devolutions#1453.

Heartbeats ride the MCS message channel framed by a Basic Security
Header carrying SEC_HEARTBEAT, exactly like auto-detect after Devolutions#1348, so
encode_heartbeat mirrors encode_autodetect_request. Per 2.2.16.1
heartbeats SHOULD only be sent when no other PDU went out in the
interval, so SharedWriter counts successful writes and the heartbeat
tick skips the send when the count moved: ordinary traffic doubles as
the liveness signal.

The defaults (period 5 seconds, warning after 3 missed heartbeats,
reconnect after 5 more) are this library's own choice; the spec mandates
no values and the client MAY ignore both counts. A zero period is bumped
to one second to keep the timer well-formed.
@github-actions github-actions Bot added needs-review A human reviewer is the current next actor risk/unknown Risk could not be determined automatically; needs maintainer-level scrutiny size/S Size: up to 199 counted lines and 5 files; exceeds XS in either measure labels Aug 30, 2026
@mamoreau-devolutions
Marc-André Moreau (mamoreau-devolutions) merged commit 8f57691 into Devolutions:master Aug 30, 2026
43 checks passed
Anton Mostovoy (antonmos) added a commit to antonmos/IronRDP that referenced this pull request Aug 30, 2026
…arge pointer capset)

Three upstream commits since the last sync, all in client_accepted's
capability loop or client_loop's body -- code this branch has never
touched (the preemption/negotiation work stays entirely within
PendingConnection/negotiate_and_authenticate and the accept-loop race,
upstream of accept_finalize where client_accepted/client_loop live):

Devolutions#1787! (Large Pointer Capability Set) -- additive to the capabilities
match in client_accepted and UpdateEncoder::new's constructor, neither
touched here.
Devolutions#1834 (EGFX flow-control + dispatch timing diagnostics) -- logging only
inside client_loop's existing futures, no signature changes.
Devolutions#1842 (periodic Heartbeat PDUs) -- adds a field + a config setter to
RdpServer and a 5th joined future inside client_loop; client_loop's own
signature gained a parameter, but its only two call sites (both inside
client_accepted) are upstream's own code, already updated by the merge.

One trivial conflict: both branches touched the same two import lines
at the top of the file (this branch added IpAddr for the anti-storm
cooldown's EvictedPeer.ip; upstream added AtomicU64 for the heartbeat
write-counter) -- kept both.

Note from Devolutions#1834's own commit message, checked rather than trusted: it
warns the workspace-wide test build was already broken on master
independent of that PR (an ironrdp-daemon call-site mismatch). Did not
reproduce here -- full cargo build --workspace succeeded clean, so
either it was already fixed by a later commit or doesn't affect a
non-test build; not chased further since it doesn't block this merge.

14/14 tests pass in both default and --features egfx (test count
unchanged -- none of the three commits touch a test module), clippy
--all-targets clean in both configs AND under CI's exact stricter
command (--features helper,__bench --locked -- -D warnings), fmt clean,
full cargo build --workspace green.
@glamberson
Greg Lamberson (glamberson) deleted the feat/server-heartbeat-emission branch August 30, 2026 15:04

This branch was previously deployed

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

Labels

needs-review A human reviewer is the current next actor risk/unknown Risk could not be determined automatically; needs maintainer-level scrutiny 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