feat(server): IPv6 dual-stack and SO_REUSEADDR for run()#1187
Merged
Benoît Cortier (CBenoit) merged 2 commits intoApr 1, 2026
Merged
Conversation
Replace TcpListener::bind() with TcpSocket for control over socket options before binding: - SO_REUSEADDR: prevents EADDRINUSE on server restart (TIME_WAIT) - IPv6 dual-stack: when bound to [::], accepts both IPv4 and IPv6 on a single socket. Address family is detected from SocketAddr. - Backlog increased from default to 1024
Greg Lamberson (glamberson)
force-pushed
the
feat/enhanced-server-listener
branch
from
1ed889d to
dda8242
Compare
Greg Lamberson (glamberson)
changed the title
There was a problem hiding this comment.
Pull request overview
This PR updates the RDP server’s run() listener setup to use tokio::net::TcpSocket so socket options can be set prior to binding, aiming to improve restart behavior and support IPv6 dual-stack binding.
Changes:
- Replace
TcpListener::bind()withTcpSocketto configure socket options beforebind(). - Enable
SO_REUSEADDRto reduceEADDRINUSElikelihood on restart. - Increase listen backlog to 1024.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replace TcpListener::bind() with TcpSocket for control over socket options before binding: SO_REUSEADDR (unix-only): Prevents EADDRINUSE on server restart. Gated behind cfg(unix) because Windows SO_REUSEADDR has different semantics that allow port hijacking. IPv6 dual-stack: Detects address family from configured SocketAddr. When IPv6, documents platform-specific IPV6_V6ONLY behavior (Linux defaults to dual-stack, Windows/BSDs may not). Backlog: Extracted to LISTENER_BACKLOG constant (1024). No new dependencies. Backward compatible.
Greg Lamberson (glamberson)
force-pushed
the
feat/enhanced-server-listener
branch
from
bdf27cd to
bb4e31b
Compare
5 tasks
Benoît Cortier (CBenoit)
approved these changes
Apr 1, 2026
Greg Lamberson (glamberson)
added a commit
to lamco-admin/IronRDP
that referenced
this pull request
May 20, 2026
Reshape the public surface introduced earlier in this PR before it lands, to avoid a future breaking touch when ironrdp-server's anyhow-removal pass arrives. Mirrors the post-Devolutions#1264 hand-rolled error pattern (Display + core::error::Error impls, no thiserror). Trait signature was `Result<bool>` (i.e. anyhow::Result through the file-level `use anyhow::Result`). Two issues: - New public surface depending on anyhow at a moment when the workspace is moving the other way (Devolutions#1277 just removed anyhow from rdpsnd-native). - `Ok(true)` vs `Ok(false)` is stringly typed at the call site: a bare bool with no semantic clue. New shape: - `CredentialDecision::{Accept, Reject}` for the binary outcome. - `CredentialValidationError` wrapping any `core::error::Error + Send + Sync` for the case where the validator backend itself failed (LDAP unreachable, PAM transport error, etc.). Manual Display + Error impls; source chains through. - `validate(&self, &Credentials) -> Result<CredentialDecision, CredentialValidationError>`. The accept/reject/backend-error trichotomy is now explicit at every call site. Rejection is no longer an error; backend failure is. Log + bail strings updated to match the trichotomy. Also addresses the API consistency note: every other configurable on `RdpServerBuilder<BuilderDone>` goes through `with_*` on the builder. Added `with_credential_validator(Option<Arc<dyn ...>>)` following the same shape as `with_connection_handler`. The post-construction `set_credential_validator` setter is kept (now takes `Option` to match the field and the builder's setter shape) for dynamic reconfiguration after `build()`. Tests added in `server.rs` cover Accept / Reject / backend-error propagation and `Arc<dyn CredentialValidator>` Send + Sync + 'static bounds. Integration with `client_accepted` is exercised through the existing acceptor-side tests once the validator is wired via the builder; no new integration test in this commit (server-side precedent per Devolutions#1181 / Devolutions#1187 / Devolutions#1281). Verification: `cargo test -p ironrdp-server --lib` (14 passed), `cargo clippy -p ironrdp-server --all-targets -- -D warnings` clean, `cargo check -p ironrdp-server` clean. Refs Devolutions#1154, Devolutions#1150, Devolutions#1155.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace
TcpListener::bind()withTcpSocketfor control over socketoptions before binding:
SO_REUSEADDR: Prevents
EADDRINUSEon server restart. The previousTcpListener::bind()doesn't set this, so the port stays inTIME_WAITfor ~60 seconds after shutdown.
IPv6 dual-stack: Detects address family from the configured
SocketAddr. When IPv6 (e.g.[::]:3389), creates a v6 socket thataccepts both IPv4 and IPv6 via dual-stack (Linux default
bindv6only=0). When IPv4, usesnew_v4()as before.Backlog: Increased from the
TcpListenerdefault to 1024.No new dependencies. Backward compatible.