Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
cristinel24 opened this issue Sep 19, 2024 · 1 comment
Open

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

cristinel24 opened this issue Sep 19, 2024 · 1 comment

Comments

@cristinel24
Copy link

cristinel24 commented Sep 19, 2024

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:

  • Resource exhaustion: Without a maximum connection limit, the server may open too many connections, leading to resource exhaustion (e.g., memory, CPU).
  • Degraded performance: With excessive open connections, the server's performance could degrade significantly, impacting response times and stability.
  • Denial of service: Malicious actors could exploit this to open many connections, causing a denial of service (DoS) attack.

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:

  • Make MAX_CONNECTIONS configurable via environment variables or server configuration options.
  • Implement connection queuing or backoff strategies if the connection limit is reached.
  • Consider exposing metrics related to active connections to help operators monitor and adjust capacity.

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
Copy link

Hello,
I'd go with a configuration option similar to #811 as I've found it very easy to use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants