[PATCH 0/1] MR1879: ntdll/tests: Avoid testing the server pipe signaled state from the main thread.
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 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1879
From: Zebediah Figura <zfigura(a)codeweavers.com> 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)); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1879
For what it's worth, I was able to easily reproduce the failure locally with the attached patch: [scratch.diff](/uploads/8c0f49463a63ed3541b678a88ad009ce/scratch.diff) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1879#note_20068
participants (2)
-
Zebediah Figura -
Zebediah Figura (＠zfigura)