salvo-rs / salvo

A powerful web framework built with a simplified design.
https://salvo.rs
Apache License 2.0
3.4k stars 208 forks source link

Issue: Lack of Control Over Maximum Number of Connections #916

Open cristinel24 opened 1 month ago

cristinel24 commented 1 month ago

Problem Description:

In the current implementation of the server within salvo_core/src/server.rs, there is no limit or control over the maximum number of concurrent connections the server can handle. The current code continuously accepts incoming socket connections without imposing any restrictions on how many connections can be processed at a time.

This can potentially lead to a situation where the server consumes excessive resources (e.g., memory, CPU), resulting in degraded performance, denial of service, or even crashing the server if too many connections are accepted simultaneously.

Code Reference:

tokio::select! {
    accepted = acceptor.accept(fuse_factory.clone()) => {
        match accepted {
            Ok(Accepted { conn, local_addr, remote_addr, http_scheme, ..}) => {
                alive_connections.fetch_add(1, Ordering::Release);

                let service = service.clone();
                let alive_connections = alive_connections.clone();
                let notify = notify.clone();
                let handler = service.hyper_handler(local_addr, remote_addr, http_scheme, conn.fusewire(), alt_svc_h3.clone());
                let builder = builder.clone();

                let force_stop_token = force_stop_token.clone();
                let graceful_stop_token = graceful_stop_token.clone();

                tokio::spawn(async move {
                    let conn = conn.serve(handler, builder, Some(graceful_stop_token.clone()));
                    tokio::select! {
                        _ = conn => {},
                        _ = force_stop_token.cancelled() => {}
                    }

                    if alive_connections.fetch_sub(1, Ordering::Acquire) == 1 {
                        if graceful_stop_token.is_cancelled() {
                            notify.notify_one();
                        }
                    }
                });
            },
            Err(e) => {
                tracing::error!(error = ?e, "accept connection failed");
            }
        }
    }
   ...
}

The above code shows that the server accepts connections in an infinite loop without checking the number of ongoing connections. The variable alive_connections is updated to track the number of active connections, but it does not prevent further connections from being accepted if the number surpasses a threshold.

Impact:

Suggested Solution:

To mitigate these risks, I propose adding a mechanism to control the maximum number of connections the server can handle concurrently. One way to implement this is by introducing a configurable limit for alive_connections, and preventing new connections from being accepted once this limit is reached.

For example, before accepting a new connection, we could check if alive_connections is below a configurable threshold:

if alive_connections.load(Ordering::Acquire) < MAX_CONNECTIONS {
    // Proceed with accepting a new connection
} else {
    // Log and potentially return an error or a backoff mechanism
}

This would allow users to specify a maximum number of concurrent connections based on their server capacity and use case, avoiding resource exhaustion and improving overall stability.


Potential Enhancements:


Conclusion:

Adding the ability to limit the number of concurrent connections will greatly improve server stability and resource management. This will protect the server from excessive load and allow for more controlled performance scaling.

Thank you for your time!

gheorghitamutu commented 1 month ago

Hello, I'd go with a configuration option similar to https://github.com/salvo-rs/salvo/issues/811 as I've found it very easy to use.