Repository: actix/actix-web (language: rust)

Issue:
Port already in use does not cause error on Windows + WSL
## Expected Behavior
When you start up the HttpServer the bind("127.0.0.1:65000")? throws an error when the port referenced is under use.

## Current Behavior
You can start up as many instances of the same application as you want (tested up to 3) and none of the later started throw error on the fact that the port is already bound. In fact, the first instance catches the requests, the other ones does not receive it. When you terminate the first instance, the second will catch the requests. And if that is terminated then the third. And when the third is terminated, then the request fails as expected.

## Steps to Reproduce (for bugs)
1. Use the barebone application code, what can be found in the examples
2. Run multiple instances

## Context
This can cause a mayor issue if you bind your port on something you don't know is already been used, for example by a running SQL server.

## Your Environment
Windows 10, WSL - Ubuntu

* Rust Version: rustc 1.41.0 (5e1a79984 2020-01-27)
* Actix Web Version: 2.0, 1.0.8

Not panic if bind failed
This only exists on windows platforms.

## Expected Behavior
Panic if can not bind to port

## Current Behavior
continue runs, but can not really listen


## Steps to Reproduce (for bugs)
1. build
2. run at least 2

## Context
My code:
```rust
HttpServer::new(|| {
    App::new()
        .service(echo)
        .service(user::login_request)
        .service(user::register_request)
        .service(user::verify_email_request)
})
.bind(("0.0.0.0", unsafe { &CONFIG }["connection"]["serverPort"].as_i64().unwrap() as u16))
.expect("Can not bind server to port").run().await.expect("Can not start server");
```


## Your Environment
Windows 10 22H2 19045.2364

* Rust Version (I.e, output of `rustc -V`): rustc 1.68.0-nightly (77429957a 2023-01-01)
* Actix-* crate(s) Version: { version = "4.2.1", features = ["rustls"] }



Discussion / hints:
That happens because the socket is opened with `SO_REUSEADDR`. Also see my comment to a socket issue also related to bind problems: https://github.com/actix/actix-web/issues/1249#issuecomment-581046322.

I'm not sure if `SO_REUSEADDR` is desireable altogether, since it won't cause an error when the port/address already is in use (and will still lead to the server not working).
I believe just using plain `TcpListener::bind` should fix that: https://github.com/actix/actix-web/compare/master...Rudi3:use_std_TcpListener. I don't want to make a PR just yet, as it's not cleaned up yet and I'm not sure if `SO_REUSEADDR` has a deeper meaning I'm not aware of.
I assume this is configured to allow seamless restarts during updates. Maybe in debug mode it should warn if there are multiple processes listening?

The behavior of this flag also varies by OS, so it should be cfg gated for consistent behavior. The more commonly used variant of this flag in Linux is SO_REUSEPORT. With that setting, the Linux kernel will actually do load balancing of connections between all of the processes which have opened the port (as long as they all use this flag and share the same UID).

More info on the behavior of the flag and the platform differences:
https://stackoverflow.com/a/14388707/1342791
@ccope thanks, that link is very comprehensive. But shouldn't we get an error then if `SO_REUSEADDR` is currently being used and it's bound to **exactly** the same address and port? 

But it's on windows, so that changes things:

> Windows only knows the SO_REUSEADDR option, there is no SO_REUSEPORT. Setting SO_REUSEADDR on a socket in Windows behaves like setting SO_REUSEPORT and SO_REUSEADDR on a socket in BSD, with one exception: A socket with SO_REUSEADDR can always bind to exactly the same source address and port as an already bound socket, even if the other socket did not have this option set when it was bound. This behavior is somewhat dangerous because it allows an application "to steal" the connected port of another application.

So if I got that right, `SO_REUSEADDR` only behaves like `SO_REUSEPORT` on Windows, which should mean that this would only work on windows and you should currently get an error on linux?

If that's right, it should definately at least have a cfg that is consistent, as you said.
@Rudi3 @ccope I can verify that information, that on Ubuntu **it will raise exception** on bound port!
This issue still occurs on windows (probably until the PR above gets merged) - the workaround below should check the availability and panic if port is not available (untested)
```rust
fn assert_address() {
    if cfg!(target_os = "windows") {
        match std::net::TcpListener::bind("localhost:8080") {
            Ok(e) => e,
            Err(e) => {
                println!("Could not open port 8080 on localhost");
                panic!(e);
            }
        };
    }
}
```
Yep, could reproduce this on my (windows 11) machine.

Happens because of this (check the line 797, we allow a re-use of the address (port)):
https://github.com/actix/actix-web/blob/17218dc6c88848938cebc560deafbf1c2184fa56/actix-web/src/server.rs#L793-L803

I don't know if this is intended, but it surely confuses a lot of people because two instances bind to the same address, but only one receives the requests.

imo we should set this to false.

@robjtede ?
The `SO_REUSEADDR` option impacts two aspects of a socket. The first is whether two sockets with conflicting addresses are allowed to bind or not. The second is if a socket in `TIME_WAIT` state prevents a new socket from binding to the same address. This is discussed in detail in [this SO answer](https://stackoverflow.com/questions/14388706/how-do-so-reuseaddr-and-so-reuseport-differ/14388707#14388707). So just setting this to false will alter the behaviour for both of the above aspects. I tested on both Windows (Win10) and Linux (Ubuntu 22.04) by inducing the server socket into the `TIME_WAIT` state and trying to bind. With `SO_REUSEADDR` set to true, both OSs allowed binding but with false only Windows allowed to bind.

So I think a better option is to set this to true only on non-Windows platforms:

```rust
fn create_tcp_listener(addr: net::SocketAddr, backlog: u32) -> io::Result<net::TcpListener> {
    use socket2::{Domain, Protocol, Socket, Type};
    let domain = Domain::for_address(addr);
    let socket = Socket::new(domain, Type::STREAM, Some(Protocol::TCP))?;
    #[cfg(not(windows))]
    {
        socket.set_reuse_address(true)?;
    }
    socket.bind(&addr.into())?;
    // clamp backlog to max u32 that fits in i32 range
    let backlog = cmp::min(backlog, i32::MAX as u32) as i32;
    socket.listen(backlog)?;
    Ok(net::TcpListener::from(socket))
}
```
This will make sure that Windows behaviour better matches with other platforms like Linux if exact same ip:port is used by two processes. Note that even after my suggested change there is a difference in behaviour between Windows and Linux if one process binds to a wildcard (e.g. 0.0.0.0:8080) and another to a specific address (127.0.0.1:8080). In this case the bind succeeds on Windows but fails on Linux, but this is more of an edge case so should be ok.

The above suggested change should also fix #1335 and #1913 

One thing to keep in mind is that this will be a breaking change on Windows. So not sure how to introduce it. Maybe in next major version? @robjtede any ideas?
@imor, thanks for this fantastic analysis! I concur -- I primarily use actix on Windows; I've been bitten by the current behavior several times. In my case, I left my server running, then also ran unit tests, which start another copy of the server. My tests failed in difficult-to-debug ways -- because **both** servers bound to the same IP address and port, but the first server (production, not unit test) handled all the traffic generated by the tests. Preventing two servers from binding to the same address is a great fix!

I tested on Windows by commenting out the line `socket.set_reuse_address(true)?;` and this prevented multiple bindings to the same port. I therefore agree with this fix and would like to see it merged.

I'd also like to thank @robjtede first for creating and maintain actix -- it's been a pleasure to work with this software! In addition, his responses on Discord to my initial questions on this topic led me here.

I thought I'd pass on a few observations I made when initially researching this topic, which are also posted on Discord, hoping they'll be helpful to others:

- On Windows, use [SO_EXCLUSIVEADDRUSE](https://learn.microsoft.com/en-us/windows/win32/winsock/so-exclusiveaddruse) instead of the SO_REUSEADDR due to Windows bugs; the SO article linked above by @imor covers this nicely. See also more [Windows docs](https://learn.microsoft.com/en-us/windows/win32/winsock/using-so-reuseaddr-and-so-exclusiveaddruse). I'm not sure which flavor the socket2 library uses. (Of course, the proposed fix does not use this flag, making this somewhat irrelevant.)
Nice to see interest on this issue. Thanks both for the research and input here.

I'm not able to effectively develop on Windows outside of test-on-CI so would appreciate a PR from someone who can test it out properly.

Fix patch:
diff --git a/actix-web/CHANGES.md b/actix-web/CHANGES.md
index 36e8b62dd42..aca4ccfaef5 100644
--- a/actix-web/CHANGES.md
+++ b/actix-web/CHANGES.md
@@ -3,6 +3,7 @@
 ## Unreleased
 
 - Minimum supported Rust version (MSRV) is now 1.75.
+- On Windows platforms, produce an error when invoking `HttpServer::bind` on a socket that's already in use. See [issue 2958](https://github.com/actix/actix-web/issues/2958).
 
 ## 4.9.0
 
diff --git a/actix-web/src/server.rs b/actix-web/src/server.rs
index d8519fb9ea2..4fbeb018661 100644
--- a/actix-web/src/server.rs
+++ b/actix-web/src/server.rs
@@ -1085,7 +1085,10 @@ fn create_tcp_listener(addr: net::SocketAddr, backlog: u32) -> io::Result<net::T
     use socket2::{Domain, Protocol, Socket, Type};
     let domain = Domain::for_address(addr);
     let socket = Socket::new(domain, Type::STREAM, Some(Protocol::TCP))?;
-    socket.set_reuse_address(true)?;
+    #[cfg(not(windows))]
+    {
+        socket.set_reuse_address(true)?;
+    }
     socket.bind(&addr.into())?;
     // clamp backlog to max u32 that fits in i32 range
     let backlog = cmp::min(backlog, i32::MAX as u32) as i32;


Test patch:
diff --git a/actix-web/tests/test_httpserver.rs b/actix-web/tests/test_httpserver.rs
index 039c0ffbc6b..5fd7d719035 100644
--- a/actix-web/tests/test_httpserver.rs
+++ b/actix-web/tests/test_httpserver.rs
@@ -1,13 +1,10 @@
 #[cfg(feature = "openssl")]
 extern crate tls_openssl as openssl;
 
-#[cfg(any(unix, feature = "openssl"))]
-use {
-    actix_web::{web, App, HttpResponse, HttpServer},
-    std::{sync::mpsc, thread, time::Duration},
-};
+use std::{sync::mpsc, thread, time::Duration};
+
+use actix_web::{web, App, HttpResponse, HttpServer};
 
-#[cfg(unix)]
 #[actix_rt::test]
 async fn test_start() {
     let addr = actix_test::unused_addr();
@@ -53,6 +50,27 @@ async fn test_start() {
     let response = client.get(host.clone()).send().await.unwrap();
     assert!(response.status().is_success());
 
+    // Attempt to start a second server using the same address.
+    let result = HttpServer::new(|| {
+        App::new().service(
+            web::resource("/").route(web::to(|| async { HttpResponse::Ok().body("test") })),
+        )
+    })
+    .workers(1)
+    .backlog(1)
+    .max_connections(10)
+    .max_connection_rate(10)
+    .keep_alive(Duration::from_secs(10))
+    .client_request_timeout(Duration::from_secs(5))
+    .client_disconnect_timeout(Duration::ZERO)
+    .server_hostname("localhost")
+    .system_exit()
+    .disable_signals()
+    .bind(format!("{}", addr));
+
+    // This should fail: the address is in use.
+    assert!(result.is_err());
+
     srv.stop(false).await;
 }