[PATCH 0/1] MR1879: ntdll/tests: Avoid testing the server pipe signaled state from the main thread.

[Zebediah Figura (＠zfigura)]

Although the test has never been observed to fail on Windows, I think the
failure is genuinely a race in the test.

File handles (like events) are signaled in order to mark that an I/O operation
has completed. In this case the I/O operation includes manipulating data on both
ends of the pipe, and as part of that may signal the other end. Internally,
however, the file handle must logically happen *after* all of this processing
has taken place, not least because (given the Windows I/O architecture) it is
the job of the I/O manager, not the IRP handler.

Since the purpose of the test is probably just to check that the file handle
will be signaled after a synchronous I/O operation has completed, we may as well
check it from the client thread, after we know for sure that it has.

Wine-Bug: https://bugs.winehq.org//show_bug.cgi?id=54078

[PATCH 1/1] ntdll/tests: Avoid testing the server pipe signaled state from the main thread.

[Zebediah Figura]

From: Zebediah Figura

Although the test has never been observed to fail on Windows, I think the
failure is genuinely a race in the test.

File handles (like events) are signaled in order to mark that an I/O operation
has completed. In this case the I/O operation includes manipulating data on both
ends of the pipe, and as part of that may signal the other end. Internally,
however, the file handle must logically happen *after* all of this processing
has taken place, not least because (given the Windows I/O architecture) it is
the job of the I/O manager, not the IRP handler.

Since the purpose of the test is probably just to check that the file handle
will be signaled after a synchronous I/O operation has completed, we may as well
check it from the client thread, after we know for sure that it has.

Wine-Bug: https://bugs.winehq.org//show_bug.cgi?id=54078
---
 dlls/ntdll/tests/pipe.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/dlls/ntdll/tests/pipe.c b/dlls/ntdll/tests/pipe.c
index 0e373dcf7ea..11192d6e471 100644
--- a/dlls/ntdll/tests/pipe.c
+++ b/dlls/ntdll/tests/pipe.c
@@ -1679,6 +1679,7 @@ static DWORD WINAPI blocking_thread(void *arg)
             ok(is_signaled(ctx->pipe), "pipe is not signaled\n");
             ret = WriteFile(ctx->pipe, buf, 1, &num_bytes, NULL);
             ok(ret, "WriteFile failed, error %lu\n", GetLastError());
+            ok(is_signaled(ctx->pipe), "pipe is not signaled\n");
             break;
         case BLOCKING_THREAD_READ:
             Sleep(100);
@@ -1689,6 +1690,7 @@ static DWORD WINAPI blocking_thread(void *arg)
             ok(is_signaled(ctx->pipe), "pipe is not signaled\n");
             ret = ReadFile(ctx->pipe, read_buf, 1, &num_bytes, NULL);
             ok(ret, "WriteFile failed, error %lu\n", GetLastError());
+            ok(is_signaled(ctx->pipe), "pipe is not signaled\n");
             break;
         case BLOCKING_THREAD_QUIT:
             return 0;
@@ -1748,7 +1750,6 @@ static void test_blocking(ULONG options)
     ok(io.Status == STATUS_SUCCESS, "Status = %lx\n", io.Status);
     ok(io.Information == 1, "Information = %Iu\n", io.Information);
     ok(is_signaled(ctx.client), "client is not signaled\n");
-    ok(is_signaled(ctx.pipe), "pipe is not signaled\n");

res = WaitForSingleObject(ctx.done, 10000);
     ok(res == WAIT_OBJECT_0, "wait returned %lx\n", res);
@@ -1767,7 +1768,6 @@ static void test_blocking(ULONG options)
     ok(is_signaled(ctx.event), "event is not signaled\n");
     todo_wine
     ok(is_signaled(ctx.client), "client is not signaled\n");
-    ok(is_signaled(ctx.pipe), "pipe is not signaled\n");

if (!(options & FILE_SYNCHRONOUS_IO_ALERT))
         ok(!ioapc_called, "ioapc called\n");
@@ -1797,7 +1797,6 @@ static void test_blocking(ULONG options)
     res = WaitForSingleObject(ctx.done, 10000);
     ok(res == WAIT_OBJECT_0, "wait returned %lx\n", res);

-    ok(is_signaled(ctx.pipe), "pipe is not signaled\n");
     CloseHandle(ctx.pipe);
     CloseHandle(ctx.client);

@@ -1806,13 +1805,11 @@ static void test_blocking(ULONG options)
                      PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE, 4096);

ok(is_signaled(ctx.client), "client is not signaled\n");
-    ok(is_signaled(ctx.pipe), "pipe is not signaled\n");

ret = WriteFile(ctx.client, read_buf, 1, &num_bytes, NULL);
     ok(ret, "WriteFile failed, error %lu\n", GetLastError());

ok(is_signaled(ctx.client), "client is not signaled\n");
-    ok(is_signaled(ctx.pipe), "pipe is not signaled\n");

ioapc_called = FALSE;
     memset(&io, 0xff, sizeof(io));

[Zebediah Figura (＠zfigura)]

For what it's worth, I was able to easily reproduce the failure locally with the attached patch:

[scratch.diff](/uploads/8c0f49463a63ed3541b678a88ad009ce/scratch.diff)