[PATCH 0/2] MR4240: server: Only set sock->event when the event is first noticed.

[PATCH 1/2] ws2_32/tests: Add a regression test for interaction of sock_reselect() with event select.

[Zebediah Figura]

From: Zebediah Figura

---
 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();

[PATCH 2/2] server: Only set sock->event when the event is first noticed.

[Zebediah Figura]

From: Zebediah Figura

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;
     }

[PATCH 2/2] server: Only set sock->event when the event is first noticed.

[Marvin]

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 ""