feat: Add Send bounds to AsyncNetworkClient (fixes #542)

[lidarbtc]

Hey Devolutions team,

First off, thanks so much for creating IronRDP! It's been incredibly useful, and I really appreciate the work you've put into it.

I wanted to propose a change that addresses issue #542 and enables using the AsyncNetworkClient more effectively in multithreaded applications, based on my experience building a Tauri app.

Motivation & Problem (#542):

As discussed in #542, the original AsyncNetworkClient lacked Send bounds. This prevented it from being used directly within spawned asynchronous tasks (e.g., using tokio::spawn). I encountered this exact limitation while building a Tauri application where I need to manage multiple simultaneous RDP connections, each running in its own Tokio task. For this to work safely, the client and its future need to be Send.

Solution:

To resolve #542 and support my use case, I've implemented a two-part solution:

1. Add Send Bounds to AsyncNetworkClient: I've added the necessary Send bounds to the main trait. This directly fixes the issue reported in error: future cannot be sent between threads safely #542 and allows the client to be used safely across threads in environments like Tokio.

   // Now requires Send for the client and the Future, enabling use in tokio::spawn
   pub trait AsyncNetworkClient: Send {
       fn send<'a>(...) -> Pin<Box<dyn Future<Output = ...> + Send + 'a>>;
   }

2. Introduce WasmAsyncNetworkClient: Simply adding Send bounds would break compatibility with WASM environments, as WASM is single-threaded and its associated types often don't implement Send. To prevent this regression, I've created a parallel trait WasmAsyncNetworkClient specifically for WASM, which is identical but omits the Send requirements.

   // No Send bounds, preserving compatibility for single-threaded WASM
   pub trait WasmAsyncNetworkClient {
       fn send<'a>(...) -> Pin<Box<dyn Future<Output = ...> + 'a>>;
   }

Validation:

This dual-trait setup is currently working successfully in my Tauri application. It allows me to spawn multiple RDP connection tasks using tokio::spawn (thanks to the Send bounds on AsyncNetworkClient), while separate WASM builds using WasmAsyncNetworkClient continue to compile and function correctly.

Proposal:

This approach directly solves the core problem of #542 by enabling thread-safe multithreaded usage, while carefully preserving WASM compatibility via the separate trait. I believe this makes the library more robust and versatile for different deployment targets.

Happy to discuss this further! Let me know your thoughts.

[CBenoit]

Hello! Thank you for the PR.

To be honest, I don’t really like the idea of duplicating the code, but I understand where you come from.

In this case, I think it’s fine to drop the object-safety because the connect_finalize functions are already subject to monomorphization on the stream, and represent relatively small helpers.

Can you try something like this?

pub trait WasmAsyncNetworkClient {
    async fn send(
        &mut self,
        network_request: &NetworkRequest,
    ) -> ConnectorResult<Vec<u8>>;
}

And

[instrument(skip_all)]
pub async fn wasm_connect_finalize<S, C>(
    _: Upgraded,
    framed: &mut Framed<S>,
    mut connector: ClientConnector,
    server_name: ServerName,
    server_public_key: Vec<u8>,
    network_client: Option<&mut C>,
    kerberos_config: Option<KerberosConfig>,
) -> ConnectorResult<ConnectionResult>
where
    S: FramedRead + FramedWrite,
    C: AsyncNetworkClient,
{
    /* … */
}

The expectation is that the monomorphized code will be Send or not based on the concrete types defined in the consumer.

note: I’m also okay with renaming AsyncNetworkClient to NetworkClient. Not sure why there is an Async prefix in there. The crate is called ironrdp-async.

[lidarbtc]

Thanks for the detailed feedback and suggestion.
I understand your point. It makes sense to avoid duplicating code.

I'll update the PR based on your recommendation.
I'll try to have the changes ready and pushed within the next few days.