fix(server): release the static channels when a connection ends by maryny4 · Pull Request #1721 · Devolutions/IronRDP

Marynych Oleksandr (maryny4) · 2026-08-20T15:49:49Z

run_connection leaves the connection's static channels attached to the
server. run clears them right after each session, so servers driven by it
are unaffected — but the channel set is per-connection state, and
run_connection is the public entry point for embedders that run their own
accept loop. There is no public API to clear it from outside.

Channel backends own real resources, and several are released through Drop:
RdpsndServer::drop stops its handler, which is how an audio backend learns
to stop capturing. Held past the connection, they keep running with no client
to serve, and their events accumulate in the server event queue until the
next client attaches new channels.

Measured on hypr-rdp, a Wayland RDP server that accepts through
run_connection so it can answer concurrent connections instead of leaving
them in the backlog. After the client was killed:

	audio capture	virtual sink	RSS over 50 s
`run()`	stopped	removed	flat
`run_connection()`	still running	still the default output	+176 KB/s

176 KB/s is exactly 44100 Hz x 2ch x 16-bit — the capture thread still
recording into the queue.

The reset moves into run_connection_with so it covers every exit path,
including the early returns, and the now-redundant one in run is dropped.
The doc comment above the same function already explains why per-connection
state has to be handled here rather than by the caller.

Test: run_connection_releases_the_static_channels inserts a channel that
flags its own drop, ends a connection at EOF, and asserts the flag. It fails
without the change.

`run_connection` left the connection's static channels attached to the server. `run` cleared them right after each session, so servers driven by it were unaffected, but the channel set is per-connection state and `run_connection` is the public entry point for embedders with their own accept loop. Channel backends own real resources, and several are released through `Drop`: `RdpsndServer::drop` stops its handler, which is how an audio backend learns to stop capturing. Held past the connection, they keep running with no client to serve, and their events accumulate in the server event queue until the next client attaches new channels. Measured on a Wayland RDP server (hypr-rdp) that accepts through `run_connection` to answer concurrent connections: after the client was killed, the PipeWire capture thread stayed alive, its virtual sink stayed installed as the machine's default output, and the process grew by 176 KB/s — the exact PCM rate of the still-running capture. With the reset, the sink disappears when the connection ends and memory is flat. Move the reset into `run_connection_with` so it covers every exit path, including the early returns, and drop the now-redundant one in `run`.

…, keyboard metadata, entropy coder) Three upstream commits since the last sync, all in crates/ironrdp-server: Devolutions#1691 (feat!, keyboard metadata via ConnectionHandler::on_connection_info) merged with no conflict -- purely additive (a new struct, a new default trait method, one call site in client_accepted, which this branch never touches). Devolutions#1721 (fix, release static channels when a connection ends) restructured run_connection_with itself, which this branch also restructured (into PendingConnection-based negotiation) -- git's merge conflicted on run()'s whole preemption-race loop and, separately, split my mod preempt_tests and Devolutions#1721's own new mod tests into two overlapping conflict hunks (same file-tail location, different module names). Resolved by taking this branch's side for the run() loop (a superset of upstream's simpler non-preemption tail -- same on_disconnected call, just wrapped in the preemption if/else) and keeping BOTH test modules side by side. One splice bug caught by the build: mod preempt_tests was left without its own closing brace; fixed. The real interaction worth documenting: Devolutions#1721's fix assumes run_connection_with is the only connection entry point, but this branch adds a second one, serve_negotiated (the preemption-winner path), which never calls run_connection_with. Removing run()'s own static_channels reset -- which upstream's fix intends, since run_connection_with now resets it universally -- would have silently reintroduced Devolutions#1721's exact leak, just scoped to every preemption takeover instead of every run_connection embedder. Kept run()'s reset, with a comment explaining why it is NOT redundant despite resetting the same field twice on the run_connection path. Devolutions#1685/Devolutions#1684 (RemoteFX quant, already merged in c831bcc) plus this sync's new Devolutions#1686 (entropy coder weighting) add a second new RdpServerOptions field, remotefx_entropy_coder: Option<EntropyBits> -- added to the one hand-built RdpServerOptions literal in preempt_tests, same as remotefx_quant before it. Verification, in order, catching problems at each step rather than trusting a clean `cargo build`: plain build was clean but does not compile #[cfg(test)] code, so `cargo test` is what actually caught the missing remotefx_entropy_coder field. 24/24 tests pass in both default and --features egfx (up from 23 -- upstream's own run_connection_releases_the_static_channels regression test now runs against this branch's PendingConnection-based negotiation and passes, which is strong evidence the merge is semantically correct, not just textually conflict-free). clippy --all-targets clean in both configs, fmt clean, full cargo build --workspace green.

…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.

Marynych Oleksandr (maryny4) temporarily deployed to llm-providers August 20, 2026 15:50 — with GitHub Actions Inactive

github-actions Bot added 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 20, 2026

Marynych Oleksandr (maryny4) force-pushed the fix/reset-static-channels-per-connection branch from 4826ff3 to cd6d9cb Compare August 20, 2026 16:49

Marynych Oleksandr (maryny4) temporarily deployed to llm-providers August 20, 2026 16:50 — with GitHub Actions Inactive

github-actions Bot added the kind/protocol Affects RDP or related protocol behavior label Aug 20, 2026

Marc-André Moreau (mamoreau-devolutions) merged commit 8b1f0da into Devolutions:master Aug 20, 2026
43 checks passed

Marynych Oleksandr (maryny4) mentioned this pull request Sep 1, 2026

chore(deps): take ironrdp from upstream instead of the fork MuNeNiCK/hypr-rdp#77

Merged

Navigation Menu

Repository navigation

fix(server): release the static channels when a connection ends - #1721

fix(server): release the static channels when a connection ends#1721
Marc-André Moreau (mamoreau-devolutions) merged 1 commit into
Devolutions:masterfrom
maryny4:fix/reset-static-channels-per-connection

Marynych Oleksandr (maryny4) commented Aug 20, 2026

Uh oh!

Uh oh!

This branch was previously deployed

Reviewers

Uh oh!

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

Conversation

Marynych Oleksandr (maryny4) commented Aug 20, 2026

Uh oh!

Uh oh!

This branch was previously deployed

Reviewers

Uh oh!

Assignees

Labels

Milestone

Development

Uh oh!

2 participants