[PATCH 1/2] ws2_32/tests: Test whether WSAEventSelect() modifies the event state.
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/ws2_32/tests/sock.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index 50f79e6adb8..db6819e77cf 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -5186,6 +5186,42 @@ static void test_accept_events(struct event_test_ctx *ctx) closesocket(server); closesocket(client); + /* As above, but select on a subset containing FD_ACCEPT first. */ + + if (!ctx->is_message) + { + select_events(ctx, listener, FD_CONNECT | FD_READ | FD_OOB | FD_ACCEPT); + + 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()); + + ret = WaitForSingleObject(ctx->event, 200); + ok(!ret, "wait timed out\n"); + + select_events(ctx, listener, FD_CONNECT | FD_READ | FD_OOB); + ret = WaitForSingleObject(ctx->event, 0); + ok(!ret, "wait timed out\n"); + + ResetEvent(ctx->event); + + select_events(ctx, listener, FD_CONNECT | FD_READ | FD_OOB); + ret = WaitForSingleObject(ctx->event, 0); + ok(ret == WAIT_TIMEOUT, "expected timeout\n"); + + select_events(ctx, listener, FD_CONNECT | FD_READ | FD_OOB | FD_ACCEPT); + ret = WaitForSingleObject(ctx->event, 0); + todo_wine ok(!ret, "wait timed out\n"); + if (!ret) + check_events(ctx, FD_ACCEPT, 0, 0); + + server = accept(listener, NULL, NULL); + ok(server != -1, "failed to accept, error %u\n", WSAGetLastError()); + closesocket(server); + closesocket(client); + } + /* As above, but select on a subset not containing FD_ACCEPT first. */ select_events(ctx, listener, FD_CONNECT | FD_READ | FD_OOB); -- 2.34.1
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52335 Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/ws2_32/tests/sock.c | 5 ++--- server/sock.c | 5 +++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index db6819e77cf..c00d25fec3e 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -5212,9 +5212,8 @@ static void test_accept_events(struct event_test_ctx *ctx) select_events(ctx, listener, FD_CONNECT | FD_READ | FD_OOB | FD_ACCEPT); ret = WaitForSingleObject(ctx->event, 0); - todo_wine ok(!ret, "wait timed out\n"); - if (!ret) - check_events(ctx, FD_ACCEPT, 0, 0); + ok(!ret, "wait timed out\n"); + check_events(ctx, FD_ACCEPT, 0, 0); server = accept(listener, NULL, NULL); ok(server != -1, "failed to accept, error %u\n", WSAGetLastError()); diff --git a/server/sock.c b/server/sock.c index 736f34feac5..c7378306511 100644 --- a/server/sock.c +++ b/server/sock.c @@ -2543,6 +2543,11 @@ 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 ); return; } -- 2.34.1
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=104952 Your paranoid android. === debian11 (64 bit WoW report) === ws2_32: sock.c:1135: Test failed: wait failed, error 258
On 1/6/22 10:03, Zebediah Figura wrote:
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52335 Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/ws2_32/tests/sock.c | 5 ++--- server/sock.c | 5 +++++ 2 files changed, 7 insertions(+), 3 deletions(-)
[snip]
diff --git a/server/sock.c b/server/sock.c index 736f34feac5..c7378306511 100644 --- a/server/sock.c +++ b/server/sock.c @@ -2543,6 +2543,11 @@ 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 );
On a second thought, I think it makes sense to follow up almost every call to sock_reselect() with a call to sock_wake_up(). Generally, an asynchronous socket object, which serves as an emulation layer between Unix non-blocking I/O and Windows asynchronous I/O, has to maintain two invariants at any poll() point (ditto for epoll, /dev/poll, or any other poll-like calls): 1. For all waiters (from WSAPoll, overlapped I/O or otherwise), the corresponding flags in the requested poll events mask are set. 2. For all flags set in the intersection (bitwise AND) of the requested poll events mask and the returned poll events mask, the correspoding waiters shall eventually be notified. (Note: assumes level-triggered events.) As you have observed, sock_reselect() serves to establish invariant 1 but in the process disrupts invariant 2, since it updates flags without taking account the implicit changes to the set of notified waiters in the process of updating the mask. In fact, I think it's sensible to merge sock_reselect() and sock_wake_up() in one function. Any thoughts?
return; }
-- Sincerely, Jinoh Kang
On 1/7/22 18:49, Jinoh Kang wrote:
On 1/6/22 10:03, Zebediah Figura wrote:
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52335 Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/ws2_32/tests/sock.c | 5 ++--- server/sock.c | 5 +++++ 2 files changed, 7 insertions(+), 3 deletions(-)
[snip]
diff --git a/server/sock.c b/server/sock.c index 736f34feac5..c7378306511 100644 --- a/server/sock.c +++ b/server/sock.c @@ -2543,6 +2543,11 @@ 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 );
On a second thought, I think it makes sense to follow up almost every call to sock_reselect() with a call to sock_wake_up().
Please disregard my previous comment. It turns out I misunderstood what sock_reselect() does. I apologize for causing confusion. -- Sincerely, Jinoh Kang
participants (3)
-
Jinoh Kang -
Marvin -
Zebediah Figura