December 2021 - Wine-Devel - list.winehq.org

[PATCH 2/2] server: Reselect the socket returned from accept_socket().

by Zebediah Figura

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52024 Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/ws2_32/tests/sock.c | 4 ++-- server/sock.c | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index ec428acaf32..19ff5a711cc 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -5246,8 +5246,8 @@ static void test_accept_events(struct event_test_ctx *ctx) server = accept(listener, NULL, NULL); ok(server != -1, "failed to accept, error %u\n", WSAGetLastError()); ctx->socket = server; - check_events_todo(ctx, FD_WRITE, 0, 200); - check_events_todo_event(ctx, 0, 0, 0); + check_events(ctx, FD_WRITE, 0, 200); + check_events(ctx, 0, 0, 0); closesocket(server); closesocket(client); diff --git a/server/sock.c b/server/sock.c index 2df4f3d3056..736f34feac5 100644 --- a/server/sock.c +++ b/server/sock.c @@ -722,6 +722,7 @@ static void complete_async_accept( struct sock *sock, struct accept_req *req ) handle = alloc_handle_no_access_check( async_get_thread( async )->process, &acceptsock->obj, GENERIC_READ | GENERIC_WRITE | SYNCHRONIZE, OBJ_INHERIT ); acceptsock->wparam = handle; + sock_reselect( acceptsock ); release_object( acceptsock ); if (!handle) { @@ -2174,6 +2175,7 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) handle = alloc_handle( current->process, &acceptsock->obj, GENERIC_READ | GENERIC_WRITE | SYNCHRONIZE, OBJ_INHERIT ); acceptsock->wparam = handle; + sock_reselect( acceptsock ); release_object( acceptsock ); set_reply_data( &handle, sizeof(handle) ); return; -- 2.34.1

Dec. 29, 2021

[PATCH 1/2] ws2_32/tests: Test that the socket returned from accept() inherits event selection parameters.

by Zebediah Figura

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/ws2_32/tests/sock.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index 46f26d32e50..ec428acaf32 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -5222,6 +5222,37 @@ static void test_accept_events(struct event_test_ctx *ctx) closesocket(client); closesocket(listener); + + /* The socket returned from accept() inherits the same parameters. */ + + listener = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP); + ok(listener != -1, "failed to create socket, error %u\n", WSAGetLastError()); + ret = bind(listener, (const struct sockaddr *)&addr, sizeof(addr)); + ok(!ret, "failed to bind, error %u\n", WSAGetLastError()); + len = sizeof(destaddr); + ret = getsockname(listener, (struct sockaddr *)&destaddr, &len); + ok(!ret, "failed to get address, error %u\n", WSAGetLastError()); + ret = listen(listener, 2); + ok(!ret, "failed to listen, error %u\n", WSAGetLastError()); + + client = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP); + ok(client != -1, "failed to create socket, error %u\n", WSAGetLastError()); + ret = connect(client, (struct sockaddr *)&destaddr, sizeof(destaddr)); + ok(!ret, "failed to connect, error %u\n", WSAGetLastError()); + + select_events(ctx, listener, FD_CONNECT | FD_READ | FD_OOB | FD_ACCEPT | FD_WRITE); + check_events(ctx, FD_ACCEPT, 0, 200); + + server = accept(listener, NULL, NULL); + ok(server != -1, "failed to accept, error %u\n", WSAGetLastError()); + ctx->socket = server; + check_events_todo(ctx, FD_WRITE, 0, 200); + check_events_todo_event(ctx, 0, 0, 0); + + closesocket(server); + closesocket(client); + + closesocket(listener); } static void test_connect_events(struct event_test_ctx *ctx) -- 2.34.1

Dec. 29, 2021

Re: [PATCH 4/4] ws2_32: Do not assume that an fd_set is bounded by FD_SETSIZE.

by Marvin

Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=104604 Your paranoid android. === debian11 (32 bit Japanese:Japan report) === ws2_32: sock.c:5201: Test failed: expected timeout

Dec. 29, 2021

Re: [PATCH 3/4] ws2_32: Don't bother checking sockets against the input write or except sets.

by Marvin

Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=104603 Your paranoid android. === debian11 (64 bit WoW report) === ws2_32: sock.c:1135: Test failed: wait failed, error 258

Dec. 29, 2021

Re: [PATCH 2/4] ws2_32/tests: Increase tolerance in a timing test.

by Marvin

Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=104602 Your paranoid android. === debian11 (32 bit Chinese:China report) === ws2_32: sock.c:1135: Test failed: wait failed, error 258

Dec. 29, 2021

Re: [PATCH 1/4] ws2_32/tests: Test selecting the same socket twice.

by Marvin

Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=104601 Your paranoid android. === debian11 (64 bit WoW report) === ws2_32: sock.c:1135: Test failed: wait failed, error 258

Dec. 29, 2021

Re: [PATCH] po: Update on Brazilian Portuguese

by Alexandre Julliard

Roberto Pungartnik <rpungartnik(a)gmail.com> writes: > #: dlls/tzres/tzres.rc:344 > -#, fuzzy > -#| msgid "(UTC+02:00) Jerusalem" > msgid "(UTC+02:00) Juba" > msgstr "(UTC+02:00) Jerusalém" That doesn't seem right. -- Alexandre Julliard julliard(a)winehq.org

Dec. 29, 2021

Re: [PATCH] wininet: Do not call InternetUnlockRequestFile when locking failed.

by Zebediah Figura (she/her)

On 12/29/21 14:40, Robert Wilhelm wrote: > This should fix crashes on W10L vms, when we pass NULL to InternetUnlockRequestFile after prior lock already failed. > Crashes are here: https://test.winehq.org/data/tests/wininet:http.html > Behavior after crash is here: https://testbot.winehq.org/JobDetails.pl?Key=104580 > > Signed-off-by: Robert Wilhelm <robert.wilhelm(a)gmx.net> > --- > dlls/wininet/tests/http.c | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/dlls/wininet/tests/http.c b/dlls/wininet/tests/http.c > index cf3f62f868a..ee56c64fbd9 100644 > --- a/dlls/wininet/tests/http.c > +++ b/dlls/wininet/tests/http.c > @@ -1718,10 +1718,13 @@ static void InternetLockRequestFile_test(void) > > ret = InternetLockRequestFile(req.request, &lock2); > ok(ret, "InternetLockRequestFile returned: %x(%u)\n", ret, GetLastError()); > + ok(lock2 != NULL, "lock2 == NULL\n"); > ok(lock == lock2, "lock != lock2\n"); > > - ret = InternetUnlockRequestFile(lock2); > - ok(ret, "InternetUnlockRequestFile failed: %u\n", GetLastError()); > + if (lock2) { > + ret = InternetUnlockRequestFile(lock2); > + ok(ret, "InternetUnlockRequestFile failed: %u\n", GetLastError()); > + } > Shouldn't we fix whatever's causing "lock2" to be NULL instead? Even with this patch we're still getting a test failure.

Dec. 29, 2021

[PATCH 4/4] ws2_32: Do not assume that an fd_set is bounded by FD_SETSIZE.

by Zebediah Figura

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52302 Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/ws2_32/socket.c | 92 +++++++++++++++++++++++------------- dlls/ws2_32/tests/sock.c | 31 +++++++++--- dlls/ws2_32/ws2_32_private.h | 1 + 3 files changed, 84 insertions(+), 40 deletions(-) diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index e517b239d5b..2b05884acaf 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -2364,13 +2364,8 @@ static int add_fd_to_set( SOCKET fd, struct fd_set *set ) return 0; } - if (set->fd_count < FD_SETSIZE) - { - set->fd_array[set->fd_count++] = fd; - return 1; - } - - return 0; + set->fd_array[set->fd_count++] = fd; + return 1; } @@ -2380,9 +2375,9 @@ static int add_fd_to_set( SOCKET fd, struct fd_set *set ) int WINAPI select( int count, fd_set *read_ptr, fd_set *write_ptr, fd_set *except_ptr, const struct timeval *timeout) { - char buffer[offsetof( struct afd_poll_params, sockets[FD_SETSIZE * 3] )] = {0}; - struct afd_poll_params *params = (struct afd_poll_params *)buffer; - struct fd_set read_input; + struct fd_set *read_input = NULL; + struct afd_poll_params *params; + unsigned int poll_count = 0; ULONG params_size, i, j; SOCKET poll_socket = 0; IO_STATUS_BLOCK io; @@ -2392,22 +2387,49 @@ int WINAPI select( int count, fd_set *read_ptr, fd_set *write_ptr, TRACE( "read %p, write %p, except %p, timeout %p\n", read_ptr, write_ptr, except_ptr, timeout ); - FD_ZERO( &read_input ); - if (read_ptr) read_input.fd_count = read_ptr->fd_count; - if (!(sync_event = get_sync_event())) return -1; + if (read_ptr) poll_count += read_ptr->fd_count; + if (write_ptr) poll_count += write_ptr->fd_count; + if (except_ptr) poll_count += except_ptr->fd_count; + + if (!poll_count) + { + SetLastError( WSAEINVAL ); + return -1; + } + + params_size = offsetof( struct afd_poll_params, sockets[poll_count] ); + if (!(params = calloc( params_size, 1 ))) + { + SetLastError( WSAENOBUFS ); + return -1; + } + if (timeout) params->timeout = timeout->tv_sec * -10000000 + timeout->tv_usec * -10; else params->timeout = TIMEOUT_INFINITE; - for (i = 0; i < read_input.fd_count; ++i) + if (read_ptr) { - params->sockets[params->count].socket = read_input.fd_array[i] = read_ptr->fd_array[i]; - params->sockets[params->count].flags = AFD_POLL_READ | AFD_POLL_ACCEPT | AFD_POLL_HUP; - ++params->count; - poll_socket = read_input.fd_array[i]; + unsigned int read_size = offsetof( struct fd_set, fd_array[read_ptr->fd_count] ); + + if (!(read_input = malloc( read_size ))) + { + free( params ); + SetLastError( WSAENOBUFS ); + return -1; + } + memcpy( read_input, read_ptr, read_size ); + + for (i = 0; i < read_ptr->fd_count; ++i) + { + params->sockets[params->count].socket = read_ptr->fd_array[i]; + params->sockets[params->count].flags = AFD_POLL_READ | AFD_POLL_ACCEPT | AFD_POLL_HUP; + ++params->count; + poll_socket = read_ptr->fd_array[i]; + } } if (write_ptr) @@ -2432,42 +2454,43 @@ int WINAPI select( int count, fd_set *read_ptr, fd_set *write_ptr, } } - if (!params->count) - { - SetLastError( WSAEINVAL ); - return -1; - } - - params_size = offsetof( struct afd_poll_params, sockets[params->count] ); + assert( params->count == poll_count ); status = NtDeviceIoControlFile( (HANDLE)poll_socket, sync_event, NULL, NULL, &io, IOCTL_AFD_POLL, params, params_size, params, params_size ); if (status == STATUS_PENDING) { if (WaitForSingleObject( sync_event, INFINITE ) == WAIT_FAILED) + { + free( read_input ); + free( params ); return -1; + } status = io.u.Status; } if (status == STATUS_TIMEOUT) status = STATUS_SUCCESS; if (!status) { /* pointers may alias, so clear them all first */ - if (read_ptr) FD_ZERO( read_ptr ); - if (write_ptr) FD_ZERO( write_ptr ); - if (except_ptr) FD_ZERO( except_ptr ); + if (read_ptr) read_ptr->fd_count = 0; + if (write_ptr) write_ptr->fd_count = 0; + if (except_ptr) except_ptr->fd_count = 0; for (i = 0; i < params->count; ++i) { unsigned int flags = params->sockets[i].flags; SOCKET s = params->sockets[i].socket; - for (j = 0; j < read_input.fd_count; ++j) + if (read_input) { - if (read_input.fd_array[j] == s - && (flags & (AFD_POLL_READ | AFD_POLL_ACCEPT | AFD_POLL_HUP | AFD_POLL_CLOSE))) + for (j = 0; j < read_input->fd_count; ++j) { - ret_count += add_fd_to_set( s, read_ptr ); - flags &= ~AFD_POLL_CLOSE; + if (read_input->fd_array[j] == s + && (flags & (AFD_POLL_READ | AFD_POLL_ACCEPT | AFD_POLL_HUP | AFD_POLL_CLOSE))) + { + ret_count += add_fd_to_set( s, read_ptr ); + flags &= ~AFD_POLL_CLOSE; + } } } @@ -2482,6 +2505,9 @@ int WINAPI select( int count, fd_set *read_ptr, fd_set *write_ptr, } } + free( read_input ); + free( params ); + SetLastError( NtStatusToWSAError( status ) ); return status ? -1 : ret_count; } diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index f1a020d1f0c..46f26d32e50 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -3204,9 +3204,8 @@ static void test_select(void) { static char tmp_buf[1024]; - SOCKET fdListen, fdRead, fdWrite; - fd_set readfds, writefds, exceptfds, *alloc_readfds; - unsigned int maxfd; + fd_set readfds, writefds, exceptfds, *alloc_fds; + SOCKET fdListen, fdRead, fdWrite, sockets[200]; int ret, len; char buffer; struct timeval select_timeout; @@ -3214,6 +3213,7 @@ static void test_select(void) select_thread_params thread_params; HANDLE thread_handle; DWORD ticks, id, old_protect; + unsigned int maxfd, i; char *page_pair; fdRead = socket(AF_INET, SOCK_STREAM, 0); @@ -3392,16 +3392,33 @@ static void test_select(void) page_pair = VirtualAlloc(NULL, 0x1000 * 2, MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE); VirtualProtect(page_pair + 0x1000, 0x1000, PAGE_NOACCESS, &old_protect); - alloc_readfds = (fd_set *)((page_pair + 0x1000) - offsetof(fd_set, fd_array[1])); - alloc_readfds->fd_count = 1; - alloc_readfds->fd_array[0] = fdRead; - ret = select(fdRead+1, alloc_readfds, NULL, NULL, &select_timeout); + alloc_fds = (fd_set *)((page_pair + 0x1000) - offsetof(fd_set, fd_array[1])); + alloc_fds->fd_count = 1; + alloc_fds->fd_array[0] = fdRead; + ret = select(fdRead+1, alloc_fds, NULL, NULL, &select_timeout); ok(ret == 1, "select returned %d\n", ret); VirtualFree(page_pair, 0, MEM_RELEASE); closesocket(fdRead); closesocket(fdWrite); + alloc_fds = malloc(offsetof(fd_set, fd_array[ARRAY_SIZE(sockets)])); + alloc_fds->fd_count = ARRAY_SIZE(sockets); + for (i = 0; i < ARRAY_SIZE(sockets); i += 2) + { + tcp_socketpair(&sockets[i], &sockets[i + 1]); + alloc_fds->fd_array[i] = sockets[i]; + alloc_fds->fd_array[i + 1] = sockets[i + 1]; + } + ret = select(0, NULL, alloc_fds, NULL, &select_timeout); + ok(ret == ARRAY_SIZE(sockets), "got %d\n", ret); + for (i = 0; i < ARRAY_SIZE(sockets); ++i) + { + ok(alloc_fds->fd_array[i] == sockets[i], "got socket %#Ix at index %u\n", alloc_fds->fd_array[i], i); + closesocket(sockets[i]); + } + free(alloc_fds); + /* select() works in 3 distinct states: * - to check if a connection attempt ended with success or error; * - to check if a pending connection is waiting for acceptance; diff --git a/dlls/ws2_32/ws2_32_private.h b/dlls/ws2_32/ws2_32_private.h index fa2f89f6e98..f6b6ecc7eba 100644 --- a/dlls/ws2_32/ws2_32_private.h +++ b/dlls/ws2_32/ws2_32_private.h @@ -19,6 +19,7 @@ #ifndef __WINE_WS2_32_PRIVATE_H #define __WINE_WS2_32_PRIVATE_H +#include <assert.h> #include <stdarg.h> #include <stdio.h> #include <stdlib.h> -- 2.34.1

Dec. 29, 2021

[PATCH 3/4] ws2_32: Don't bother checking sockets against the input write or except sets.

by Zebediah Figura

We only need to check the input set for CLOSE. For all other flags we can guarantee that the socket was included in a unique input set. E.g. the presence of WRITE in the output flags implies that the socket must have been in the "write" set to begin with. Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/ws2_32/socket.c | 60 +++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 32 deletions(-) diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index 318e1dac437..e517b239d5b 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -2382,7 +2382,7 @@ int WINAPI select( int count, fd_set *read_ptr, fd_set *write_ptr, { char buffer[offsetof( struct afd_poll_params, sockets[FD_SETSIZE * 3] )] = {0}; struct afd_poll_params *params = (struct afd_poll_params *)buffer; - struct fd_set read, write, except; + struct fd_set read_input; ULONG params_size, i, j; SOCKET poll_socket = 0; IO_STATUS_BLOCK io; @@ -2392,12 +2392,8 @@ int WINAPI select( int count, fd_set *read_ptr, fd_set *write_ptr, TRACE( "read %p, write %p, except %p, timeout %p\n", read_ptr, write_ptr, except_ptr, timeout ); - FD_ZERO( &read ); - FD_ZERO( &write ); - FD_ZERO( &except ); - if (read_ptr) read.fd_count = read_ptr->fd_count; - if (write_ptr) write.fd_count = write_ptr->fd_count; - if (except_ptr) except.fd_count = except_ptr->fd_count; + FD_ZERO( &read_input ); + if (read_ptr) read_input.fd_count = read_ptr->fd_count; if (!(sync_event = get_sync_event())) return -1; @@ -2406,28 +2402,34 @@ int WINAPI select( int count, fd_set *read_ptr, fd_set *write_ptr, else params->timeout = TIMEOUT_INFINITE; - for (i = 0; i < read.fd_count; ++i) + for (i = 0; i < read_input.fd_count; ++i) { - params->sockets[params->count].socket = read.fd_array[i] = read_ptr->fd_array[i]; + params->sockets[params->count].socket = read_input.fd_array[i] = read_ptr->fd_array[i]; params->sockets[params->count].flags = AFD_POLL_READ | AFD_POLL_ACCEPT | AFD_POLL_HUP; ++params->count; - poll_socket = read.fd_array[i]; + poll_socket = read_input.fd_array[i]; } - for (i = 0; i < write.fd_count; ++i) + if (write_ptr) { - params->sockets[params->count].socket = write.fd_array[i] = write_ptr->fd_array[i]; - params->sockets[params->count].flags = AFD_POLL_WRITE; - ++params->count; - poll_socket = write.fd_array[i]; + for (i = 0; i < write_ptr->fd_count; ++i) + { + params->sockets[params->count].socket = write_ptr->fd_array[i]; + params->sockets[params->count].flags = AFD_POLL_WRITE; + ++params->count; + poll_socket = write_ptr->fd_array[i]; + } } - for (i = 0; i < except.fd_count; ++i) + if (except_ptr) { - params->sockets[params->count].socket = except.fd_array[i] = except_ptr->fd_array[i]; - params->sockets[params->count].flags = AFD_POLL_OOB | AFD_POLL_CONNECT_ERR; - ++params->count; - poll_socket = except.fd_array[i]; + for (i = 0; i < except_ptr->fd_count; ++i) + { + params->sockets[params->count].socket = except_ptr->fd_array[i]; + params->sockets[params->count].flags = AFD_POLL_OOB | AFD_POLL_CONNECT_ERR; + ++params->count; + poll_socket = except_ptr->fd_array[i]; + } } if (!params->count) @@ -2459,9 +2461,9 @@ int WINAPI select( int count, fd_set *read_ptr, fd_set *write_ptr, unsigned int flags = params->sockets[i].flags; SOCKET s = params->sockets[i].socket; - for (j = 0; j < read.fd_count; ++j) + for (j = 0; j < read_input.fd_count; ++j) { - if (read.fd_array[j] == s + if (read_input.fd_array[j] == s && (flags & (AFD_POLL_READ | AFD_POLL_ACCEPT | AFD_POLL_HUP | AFD_POLL_CLOSE))) { ret_count += add_fd_to_set( s, read_ptr ); @@ -2472,17 +2474,11 @@ int WINAPI select( int count, fd_set *read_ptr, fd_set *write_ptr, if (flags & AFD_POLL_CLOSE) status = STATUS_INVALID_HANDLE; - for (j = 0; j < write.fd_count; ++j) - { - if (write.fd_array[j] == s && (flags & AFD_POLL_WRITE)) - ret_count += add_fd_to_set( s, write_ptr ); - } + if (flags & AFD_POLL_WRITE) + ret_count += add_fd_to_set( s, write_ptr ); - for (j = 0; j < except.fd_count; ++j) - { - if (except.fd_array[j] == s && (flags & (AFD_POLL_OOB | AFD_POLL_CONNECT_ERR))) - ret_count += add_fd_to_set( s, except_ptr ); - } + if (flags & (AFD_POLL_OOB | AFD_POLL_CONNECT_ERR)) + ret_count += add_fd_to_set( s, except_ptr ); } } -- 2.34.1

Dec. 29, 2021