[PATCH v6 0/1] MR3367: server: Ignore EADDRINUSE from listen() unless the provided socket is wildcarded.
This fixes a `todo_wine` in ws2_32's socket tests.
-- v6: server: Defer listens on addresses currently bound first by another socket.
From: Ally Sommers dropbear.sh@gmail.com
--- dlls/ws2_32/tests/sock.c | 2 +- server/sock.c | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index 31dff1a11c7..1278dfb0fa3 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -2298,7 +2298,7 @@ static void test_reuseaddr(void) rc = listen(s1, 1); ok(!rc, "got error %d.\n", WSAGetLastError()); rc = listen(s2, 1); - todo_wine ok(!rc, "got error %d.\n", WSAGetLastError()); + ok(!rc, "got error %d.\n", WSAGetLastError()); rc = connect(s3, tests[i].addr_loopback, tests[i].addrlen); ok(!rc, "got error %d.\n", WSAGetLastError());
diff --git a/server/sock.c b/server/sock.c index ac55a5448f7..80d314946a7 100644 --- a/server/sock.c +++ b/server/sock.c @@ -193,6 +193,7 @@ struct bound_addr union unix_sockaddr addr; int match_any_addr; int reuse_count; + struct sock *sock; };
#define MAX_ICMP_HISTORY_LENGTH 8 @@ -264,6 +265,9 @@ struct sock unsigned int reset : 1; /* did we get a TCP reset? */ unsigned int reuseaddr : 1; /* winsock SO_REUSEADDR option value */ unsigned int exclusiveaddruse : 1; /* winsock SO_EXCLUSIVEADDRUSE option value */ + unsigned int listen_queued : 1; /* listen() while bound to an in-use address? */ + int listen_backlog; /* stored argument for deferred listen() */ + struct list bind_list; /* ordered list of sockets bound to the same address */ };
static int is_tcp_socket( struct sock *sock ) @@ -397,10 +401,12 @@ static struct bound_addr *register_bound_address( struct sock *sock, const union return NULL; } ++bound_addr->reuse_count; + list_add_tail(&bound_addr->sock->bind_list, &sock->bind_list); } else { bound_addr->reuse_count = sock->reuseaddr ? 1 : -1; + bound_addr->sock = sock; } return bound_addr; } @@ -1663,7 +1669,28 @@ static int sock_close_handle( struct object *obj, struct process *process, obj_h
if (signaled) complete_async_poll( poll_req, STATUS_SUCCESS ); } + + list_remove(&sock->bind_list); + + if (sock->bound_addr[0] && sock->bound_addr[0]->sock == sock) + { + struct sock *new_sock; + struct list *next_list_elem = list_next( &sock->bind_list, &sock->bind_list ); + if (!next_list_elem) + goto no_next_socket_or_deferred_listen; + + new_sock = LIST_ENTRY( next_list_elem, struct sock, bind_list ); + sock->bound_addr[0]->sock = new_sock; + if (sock->bound_addr[1]) + sock->bound_addr[1]->sock = new_sock; + + close( get_unix_fd( sock->fd ) ); + if (new_sock->listen_queued) + if (listen( get_unix_fd( new_sock->fd ), new_sock->listen_backlog ) == -1 && debug_level) + fprintf( stderr, "FIXME: deferred listen threw errno %d (%s)\n", errno, strerror(errno) ); + } } +no_next_socket_or_deferred_listen: return async_close_obj_handle( obj, process, handle ); }
@@ -1740,6 +1767,8 @@ static struct sock *create_socket(void) sock->rcvtimeo = 0; sock->sndtimeo = 0; sock->icmp_fixup_data_len = 0; + sock->listen_queued = 0; + sock->listen_backlog = 0; sock->bound_addr[0] = sock->bound_addr[1] = NULL; init_async_queue( &sock->read_q ); init_async_queue( &sock->write_q ); @@ -1749,6 +1778,7 @@ static struct sock *create_socket(void) init_async_queue( &sock->poll_q ); memset( sock->errors, 0, sizeof(sock->errors) ); list_init( &sock->accept_list ); + list_init( &sock->bind_list ); return sock; }
@@ -2539,12 +2569,20 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) return; }
+ if (sock->bound_addr[0] && sock->bound_addr[0]->sock != sock) + { + sock->listen_backlog = params->backlog; + sock->listen_queued = 1; + goto listen_skip_error; + } + if (listen( unix_fd, params->backlog ) < 0) { set_error( sock_get_ntstatus( errno ) ); return; }
+ listen_skip_error: sock->state = SOCK_LISTENING;
/* a listening socket can no longer be accepted into */
What is the purpose if this patch, does it fix any real application?
I ran into this with a program I was writing myself; I have not encountered this issue in any existing programs.
The meaning of that test is that with SO_REUSADDR on Windows multiple sockets can be bound to the same address, and a newly bound socket may actually receive connection on it, either at once or when the other socket will be closed.
The behavior is stated to be indeterminate when two sockets are bound to the same interface and port, unless they are multicast sockets. However, it appears to be consistent that the socket to bind first is prioritized for most operations.
Maybe it is possible to achieve with Linux SO_REUSEPORT and BPF filters, but probably not by ignoring an error on listen.
Certainly the patch was insufficient at first. I didn't quite understand how `SO_REUSEADDR` works on Windows and made assumptions based on the unit tests. `SO_REUSEPORT` would unfortunately not fully work because it explicitly avoids specific behaviors that Windows allows.
---
After experimenting a lot with various edge cases, the patch in its current state does lack some behavior that Windows exhibits, most notably allowing binding to an address after another socket has begun listening on it. I still need to write tests and ensure that send and recv work accurately, but I have a good idea of how to accommodate this quirk.
I hesitate to add this complexity just to fix a todo.
Fwiw my thoughts too. If it is not really implementing that behaviour correctly and we don't know that something in the wild is helped by this partial improvement not apparent that the complexity worth it.
-
Ally Sommers -
Ally Sommers (＠dropbear)
-
Paul Gofman (＠gofman)
-
Zebediah Figura (＠zfigura)