fix(server): drop the previous session's queued events before every connection

[Greg Lamberson (glamberson)]

Summary

• discard_stale_session_events ran only before serving a preemption winner. After an ordinary disconnect, anything the session had queued but not consumed (EGFX frames, RDPSND waves, clipboard messages) stayed on the server-global event channel. run()'s idle select discards what it reads between connections, but a client reconnecting within milliseconds can win that race, and an embedder driving connections through run_connection has no idle loop at all.
• Seen on a live server: a client dropped mid-write and reconnected 45 ms later. The new connection's first write, 6 ms after its client loop started, was the previous session's 8.4 MB EGFX frame, framed for that session's graphics DVC while the new DRDYNVC had not opened any channel yet. The client failed with "access to non existing DVC channel" and dropped again.
• The discard now also runs at the start of every connection, before negotiation. Events the new session produces come after its channels start, so none of them are affected. The same allowlist applies: Quit, GetLocalAddr, SetCredentials, SetAutoReconnectCookie survive.

Validation

cargo xtask check fmt/lints/tests/typos/locks all pass. New end-to-end test a_new_connection_starts_without_the_previous_sessions_events in ironrdp-testsuite-extra queues a Disconnect event before the connection, serves one connection through run_connection with no run() idle loop in between, and checks that a real client still completes an echo round trip; it fails without the change.

Notes

An embedder that queues a per-session event before calling run_connection now has it dropped along with the stale ones; only the lifecycle allowlist carries into a connection, as it already did for a preemption winner.

[github-actions]

PR 2034 adds a single call to the existing allowlist-based discard_stale_session_events() at the top of run_connection_inner, before negotiation, so stale per-session events left by a replaced session can no longer leak into a reconnecting client or an embedder-driven connection with no idle loop. Verified against head: the helper's allowlist (Quit/GetLocalAddr/SetCredentials/SetAutoReconnectCookie) is intact; the preemption-winner call site in run() is distinct and not double-drained since winners resume via finalize_negotiated, bypassing run_connection_inner. The new regression test is valid. The fix is correct, minimal, and protocol-improving. Of two low-severity candidates, the missing contract documentation on public run_connection/run_connection_with is accepted; the comment-compression suggestion is a stylistic preference that would lose unique call-site motivation, so it is rejected.

[github-actions]

The fix is narrow and correct: the existing allowlist-based drain now also runs at the start of run_connection_inner, before negotiation, closing the reconnect race where a previous session's queued per-session events (EGFX frames, RDPSND waves, clipboard) were delivered to a replacement connection, including embedder-driven connections with no run() idle loop. The public API change for embedders queuing via event_sender() before run_connection is explicitly documented, and a new e2e regression test exercises the embedder path through run_connection. No protocol or wire-format behavior changes. Remaining concerns are minor: the one-shot drain is not a producer-quiescence guarantee (kept as a low-severity open question), and the test changes duplicate an existing echo driver, a generic harness signature, and rationale text that already appears twice in server.rs.

[github-actions]

This pull request may overlap with #2059.

Both touch the connection lifecycle in ironrdp-server's server.rs around session replacement: this PR moves the stale-event drain to before negotiation and documents what is dropped with the previous session, while #2059 splits per-connection state out of RdpServer and handles what is dropped with a connection. Shared scope in the same code region warrants human review.

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

[Benoît Cortier (CBenoit)]

LGTM, thank you!

There is a conflict with master that needs to be addressed.

Note to agent: I approved the fix, and it’s okay to merge on my behalf if the next operation does nothing else than fixing the conflict. Escalate to me if there is a significant change I should know.

[Greg Lamberson (glamberson)]

Rebased onto master. Besides the conflict in e2e.rs (master's connect_client and local_addr_of helpers replace the inline handshake, and the direct_addr branch is kept), one thing needed a change that isn't a pure conflict: master's new server_udp_addr parameter on client_server_with_connector landed inside the function this branch had split into client_server_impl, so I forwarded it through the wrapper (3 lines). Everything else is unchanged.