fix(server)!: split per-connection state

[uchouT (uchouT)]

Part of #1978: : the state-ownership step discussed there

Per-connection state moves out of RdpServer fields into a ConnectionState, created in finalize_negotiated (both entry paths reach it) and dropped with the connection. It holds the static channels, the heartbeat flag, the auto-detect manager, and the URBDRC interface allocator and request router. The manual resets go away, and the auto-detect baseline RTT no longer carries over between connections.

Breaking: removes RdpServer::get_svc_processor, get_channel_id_by_type and rtt_snapshot. They can only be called between connections, where they returned nothing or the previous connection's figures.

Next: a per-connection context for RdpServerDisplay::updates(), as discussed.

cc Greg Lamberson (@glamberson)

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[github-actions]

This pull request may overlap with #2073.

Both pull requests restructure per-connection state in ironrdp-server: #2073 moves auto-detect handles and connection-specific data into a per-connection DisplayContext, while this PR introduces a ConnectionState struct holding the auto-detect manager, static channels and UDP migration flags per connection, also replacing manual resets at connection start.

This notice is advisory only. Automated review continues as usual, and how these pull requests relate is for maintainers and authors to decide.

Note

LLM-assisted content (no human feedback).

[github-actions]

The PR moves per-connection state (static channels, heartbeat flag, AutoDetectManager, URBDRC allocator/router) from RdpServer fields into a ConnectionState created in finalize_negotiated and dropped on every exit path. Both entry paths funnel through finalize_negotiated (server.rs:2068, 2200), so the deleted manual resets in run_connection_with and the run-loop completion handler are covered structurally; the USB factory/borrow-splitting and heartbeat-flag plumbing check out; the release-then-Acquire ordering on the bandwidth-generation reset matches the documented contract. No wire-protocol or correctness defect found. The three removed public APIs have no remaining in-repo callers. Remaining publishable items are low severity: a deleted #1721 regression test leaves no lifecycle-level coverage of channel-backend release, min/max/avg RTT statistics are now computed but unreachable by embedders while the enable_autodetect doc redirects to handles that do not expose them, and the bandw…

1. [skeptical] #1721 regression test for static-channel backend release deleted without replacement — low 🟡 — crates/ironrdp-server/src/server.rs
   The deleted test was the executable guard for the #1721 leak (channel backends such as rdpsnd audio capture held open until the next client). The new ConnectionState local to finalize_negotiated makes the release invariant compiler-enforced on all return paths, so the mechanism deletion is defensible, but no remaining test verifies end-to-end that resource-owning backends are dropped when a connection ends, including via serve_negotiated (the preemption path the removed run-loop reset comment explicitly called out). A cheap lifecycle-level test (Drop-flag channel backend released after run_connection_with on an early-EOF stream) would preserve that coverage.

[github-actions]

[skeptical] min/max/avg RTT statistics no longer reachable by embedders; enable_autodetect doc redirect omits the gap — low 🟡 — Removing rtt_snapshot() is justified for its stale between-connection values, but it was the only path to RttSnapshot's min/max/avg over the RTT window. Verified in the head tree: AutoDetectManager::snapshot() is pub yet the manager now lives only in the private ConnectionState, and the still-pub RttSnapshot type can no longer be produced by any embedder-facing API. The three handles the rewritten enable_autodetect doc points to publish only latest RTT, session-lowest RTT, and bandwidth. If the statistics loss was intended, the doc should say so; otherwise a per-connection stats read path is needed.

[github-actions]

[code-compressor] Bandwidth-generation contract restated in three doc comments that must be edited in lockstep — low 🟡 — This PR adds the same 'increments every time the bandwidth figure is republished ... and when a new connection resets the figure to u32::MAX' sentence to three docs: with_autodetect_bandwidth_generation_handle (builder.rs:424-427), the autodetect_bandwidth_generation field doc (server.rs:753-756), and autodetect_bandwidth_generation_handle (server.rs:1842-1845). The copies already differ in wording (only the field doc cross-references the None case). Keeping the full contract once at the canonical field doc and reducing the builder/method docs to a one-line summary plus a rustdoc link would avoid three coordinated edits whenever the counter's trigger points change. Purely optional, no behavior impact.

[Greg Lamberson (glamberson)]

Thank you for taking this step, and for keeping it to state ownership. It is the first step from #1978 as we agreed there, and the split matches the partition I described. Leaving out display_suppressed, the auto-detect handles and the event channel is the right scope for now, since they belong with the updates() context step.

On the auto-detect handles, advancing the bandwidth generation counter on reset is correct. An embedder that rereads the bandwidth only when the generation moves would otherwise keep the previous connection's figure. lamco-rdp-server passes the generation with the value to its bandwidth estimator, so it depends on this.

Some per-connection state from my open PRs would belong in ConnectionState. #2046 adds auto_reconnect_issued, which is per connection by design. #2034 drops the previous session's queued events before every connection. #1954 keeps a pending UDP accept task and transport that are orphaned across a Deactivation-Reactivation pass today, which is the activation-scoped state #1978 describes. As I said on #1978, I will rebase my PRs onto your struct once it lands. #2048 makes the same auto-detect reset as this PR, so I am closing it in favour of this one.

[uchouT (uchouT)]

Resolved the conflict introduced by #1954 , moved connection level field into ConnectionState

[github-actions]

The PR moves per-connection state (static channels, heartbeat capability flag, auto-detect manager, USB manager, Soft-Sync/multitransport gating, UDP accept abort handle, early tunnel payloads) from RdpServer fields into a ConnectionState created and dropped in finalize_negotiated, which both entry paths (run_connection_inner and serve_negotiated) reach, and resets the embedder-facing auto-detect handles plus generation per connection. Verified in pr-head: the refactor is sound, the #1721 channel-release property becomes structural rather than manual, the auto-detect reset advances the generation so cached readers cannot keep the previous connection's figure, and Soft-Sync migration gating semantics are unchanged (no protocol impact). Three maintainability/documentation issues are confirmed in the head tree: the only executable regression test for the #1721 channel-backend release was deleted with no replacement anywhere in the tree, the new per-connection handle reset is documented o…

1. [skeptical + code-compressor] Regression test for the #1721 channel-backend release deleted with no replacement — medium 🟠 — crates/ironrdp-server/src/server.rs
   The PR deletes run_connection_releases_the_static_channels (the ResourceChannel/Drop test guarding the #1721 leak where static-channel backends, e.g. rdpsnd audio capture, stayed live until the next client) along with its mod tests block, because it poked the now-private static_channels field. A tree-wide search finds no replacement: the new autodetect test covers only handle resets, and no test exercises backend Drop on the run_connection_with or preemption serve_negotiated path. The release guarantee is now asserted only in comments on ConnectionState and finalize_negotiated, so a future edit that moves channel state out of ConnectionState or adds a path bypassing finalize_negotiated would silently reintroduce the leak. The property stays testable via a static_channel_factory that registers a Drop flag; the PR should carry a replacement regression test.
2. [skeptical] New per-connection auto-detect handle reset documented only on private fields — low 🟡 — crates/ironrdp-server/src/server.rs
   finalize_negotiated now resets autodetect_rtt, autodetect_baseline_rtt and autodetect_bandwidth to u32::MAX and bumps autodetect_bandwidth_generation on every connection (server.rs 2314-2324) — a deliberate change to what embedders observe through the public handles. Only the private-field docs describe it. The public docs still say 'u32::MAX until the first measurement completes' (autodetect_bandwidth_handle, 1911-1917), describe only the Release-after-store pairing (generation handle, 1922-1931), and still call baseline RTT 'session-lifetime lowest RTT' that 'never rises' (1898-1906), which is now per-connection. The builder docs for with_autodetect_bandwidth_handle / with_autodetect_bandwidth_generation_handle (builder.rs 415-435, touched by this PR) omit the reset and the generation-handle doc even lost its description of what increments the counter. Since the PR's stated purpose is to change cross-connection semantics for exactly these handles, the public contract should state that each connection starts at the sentinel and that the reset advances the generation.

[Benoît Cortier (CBenoit)]

PR automation is failing because of a picky-krb 0.12.5 incompatibility, fixed on master by #2074. Please rebase on master to fix it.

[uchouT (uchouT)]

Benoît Cortier (@CBenoit) rebased, but PR automation still failed.

[Benoît Cortier (CBenoit)]

It should be fixed now, but you already got the two AI reviews, so reviewing on my side now.

Thank you for cleaning up the API 🧹

Much better this way.