[PATCH 0/2] MR4240: server: Only set sock->event when the event is first noticed.
From: 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 9f6cd487777..5e22496e6ef 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -14002,6 +14002,36 @@ static void test_tcp_sendto_recvfrom(void) closesocket(server); } +/* Regression test for an internal bug affecting wget.exe. */ +static void test_select_after_WSAEventSelect(void) +{ + SOCKET client, server; + HANDLE event; + int ret; + + tcp_socketpair(&client, &server); + event = CreateEventA(NULL, FALSE, FALSE, NULL); + + ret = WSAEventSelect(client, event, FD_READ); + ok(!ret, "got error %u\n", WSAGetLastError()); + + ret = send(server, "data", 4, 0); + ok(ret == 4, "got %d\n", ret); + + ret = WaitForSingleObject(event, 1000); + ok(!ret, "got %d\n", ret); + + /* Poll. This must not trigger any events to be signalled again. */ + check_poll(client, POLLRDNORM | POLLWRNORM); + + ret = WaitForSingleObject(event, 0); + todo_wine ok(ret == WAIT_TIMEOUT, "got %d\n", ret); + + CloseHandle(event); + closesocket(server); + closesocket(client); +} + START_TEST( sock ) { int i; @@ -14061,6 +14091,7 @@ START_TEST( sock ) test_write_watch(); test_events(); + test_select_after_WSAEventSelect(); test_ipv6only(); test_TransmitFile(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4240
From: Zebediah Figura <zfigura(a)codeweavers.com> Do not set it every time another event is polled. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55838 --- dlls/ws2_32/tests/sock.c | 2 +- server/sock.c | 36 ++++++++++++++++++++++-------------- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index 5e22496e6ef..f9cc1482c41 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -14025,7 +14025,7 @@ static void test_select_after_WSAEventSelect(void) check_poll(client, POLLRDNORM | POLLWRNORM); ret = WaitForSingleObject(event, 0); - todo_wine ok(ret == WAIT_TIMEOUT, "got %d\n", ret); + ok(ret == WAIT_TIMEOUT, "got %d\n", ret); CloseHandle(event); closesocket(server); diff --git a/server/sock.c b/server/sock.c index 45a4955d603..f774cded733 100644 --- a/server/sock.c +++ b/server/sock.c @@ -787,18 +787,11 @@ static unsigned int afd_poll_flag_to_win32( unsigned int flags ) return ret; } -/* wake anybody waiting on the socket event or send the associated message */ -static void sock_wake_up( struct sock *sock ) +static void post_sock_messages( struct sock *sock ) { unsigned int events = sock->pending_events & sock->mask; int i; - if (sock->event) - { - if (debug_level) fprintf(stderr, "signalling events %x ptr %p\n", events, sock->event ); - if (events) - set_event( sock->event ); - } if (sock->window) { if (debug_level) fprintf(stderr, "signalling events %x win %08x\n", events, sock->window ); @@ -1285,6 +1278,9 @@ static void post_socket_event( struct sock *sock, enum afd_poll_bit event_bit ) { sock->pending_events |= event; sock->reported_events |= event; + + if ((sock->mask & event) && sock->event) + set_event( sock->event ); } } @@ -1329,7 +1325,7 @@ static void sock_dispatch_events( struct sock *sock, enum connection_state prevs break; } - sock_wake_up( sock ); + post_sock_messages( sock ); } static void sock_poll_event( struct fd *fd, int event ) @@ -2858,11 +2854,23 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) sock->nonblocking = 1; sock_reselect( sock ); - /* Explicitly wake the socket up if the mask causes it to become - * signaled. Note that reselecting isn't enough, since we might already - * have had events recorded in sock->reported_events and we don't want - * to select for them again. */ - sock_wake_up( sock ); + + /* Explicitly wake the socket up if the mask matches pending_events. + * + * The logic here is a bit surprising. We always set the event if the + * socket has events that haven't been consumed by + * WSAEnumNetworkEvents() yet, including if WSAEventSelect() is called + * multiple times without consuming the events. + * However, once the events are consumed by WSAEnumNetworkEvents(), we + * don't set the event again (even though e.g. data is still available) + * until a "reset" call (i.e. that clears reported_events). */ + + if (event && (sock->pending_events & mask)) + { + if (debug_level) fprintf( stderr, "signalling pending events %#x due to event select\n", + sock->pending_events & mask ); + set_event( event ); + } return; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4240
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=139417 Your paranoid android. === w1064_2qxl (64 bit report) === ws2_32: sock.c:13036: Test failed: wait timed out sock.c:13049: Test failed: got size 5 sock.c:13050: Test failed: got "datad" sock.c:12884: Test failed: got size 0 sock.c:12885: Test failed: got ""
participants (3)
-
Marvin -
Zebediah Figura -
Zebediah Figura (＠zfigura)