Signed-off-by: Brendan Shanks bshanks@codeweavers.com --- dlls/kernel32/tests/pipe.c | 108 +++++++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+)
diff --git a/dlls/kernel32/tests/pipe.c b/dlls/kernel32/tests/pipe.c index f61d441303..227575ece3 100644 --- a/dlls/kernel32/tests/pipe.c +++ b/dlls/kernel32/tests/pipe.c @@ -3887,6 +3887,113 @@ static void test_wait_pipe(void) CloseHandle(ov.hEvent); }
+static DWORD CALLBACK test_nowait_read_thread(LPVOID arg) +{ + HANDLE *pipewrite = arg; + static const char buf[] = "bits"; + DWORD written; + + Sleep(2000); + WriteFile(*pipewrite, buf, sizeof(buf), &written, NULL); + return 0; +} +static DWORD CALLBACK test_nowait_write_thread(LPVOID arg) +{ + HANDLE *piperead = arg; + char buf[32768]; + DWORD read; + + Sleep(2000); + ReadFile(*piperead, buf, sizeof(buf), &read, NULL); + return 0; +} +static DWORD CALLBACK test_nowait_connect_thread(LPVOID arg) +{ + HANDLE hFile; + + Sleep(2000); + hFile = CreateFileA(PIPENAME, GENERIC_READ | GENERIC_WRITE, 0, NULL, OPEN_EXISTING, 0, 0); + ok(hFile != INVALID_HANDLE_VALUE, "CreateFileA failed\n"); + ok(CloseHandle(hFile), "CloseHandle failed\n"); + return 0; +} +static void test_nowait(void) +{ + SECURITY_ATTRIBUTES pipe_attr; + HANDLE piperead, pipewrite, hThread; + DWORD mode; + DWORD read, write; + char readbuf[32768]; + + pipe_attr.nLength = sizeof(SECURITY_ATTRIBUTES); + pipe_attr.bInheritHandle = TRUE; + pipe_attr.lpSecurityDescriptor = NULL; + + /* CreatePipe, use SetNamedHandleState to enable PIPE_NOWAIT, and read from empty pipe */ + ok(CreatePipe(&piperead, &pipewrite, &pipe_attr, 0) != 0, "CreatePipe failed\n"); + mode = PIPE_NOWAIT; + ok(SetNamedPipeHandleState(piperead, &mode, NULL, NULL) != 0, "SetNamedPipeHandleState failed\n"); + /* spawn a thread that writes to the pipe after 2 seconds so the test won't hang if ReadFile blocks */ + hThread = CreateThread(NULL, 0, test_nowait_read_thread, &pipewrite, 0, NULL); + ok(hThread != NULL, "CreateThread failed. %d\n", GetLastError()); + SetLastError(0xdeadbeef); + todo_wine ok(ReadFile(piperead,readbuf,sizeof(readbuf),&read, NULL) == FALSE, "ReadFile should fail\n"); + todo_wine ok(GetLastError() == ERROR_NO_DATA, "got %d should be ERROR_NO_DATA\n", GetLastError()); + ok(WaitForSingleObject(hThread,INFINITE) == WAIT_OBJECT_0, "WaitForSingleObject\n"); + ok(CloseHandle(hThread), "CloseHandle for the thread failed\n"); + ok(CloseHandle(pipewrite), "CloseHandle for the write pipe failed\n"); + ok(CloseHandle(piperead), "CloseHandle for the read pipe failed\n"); + + /* CreatePipe, use SetNamedHandleState to enable PIPE_NOWAIT, and write to full pipe */ + ok(CreatePipe(&piperead, &pipewrite, &pipe_attr, 512) != 0, "CreatePipe failed\n"); + mode = PIPE_NOWAIT; + ok(SetNamedPipeHandleState(pipewrite, &mode, NULL, NULL) != 0, "SetNamedPipeHandleState failed\n"); + /* spawn a thread that reads from the pipe after 2 seconds so the test won't hang if WriteFile blocks */ + hThread = CreateThread(NULL, 0, test_nowait_write_thread, &piperead, 0, NULL); + ok(hThread != NULL, "CreateThread failed. %d\n", GetLastError()); + SetLastError(0xdeadbeef); + ok(WriteFile(pipewrite,readbuf,sizeof(readbuf),&write, NULL) == TRUE, "WriteFile should return true\n"); + /* WriteFile only documents that 'write < sizeof(readbuf)' for this case, but Windows + * doesn't seem to do partial writes ('write == 0' always) + */ + todo_wine ok(write < sizeof(readbuf), "WriteFile should fail to write the whole buffer\n"); + todo_wine ok(write == 0, "WriteFile shouldn't do partial writes\n"); + ok(CloseHandle(hThread), "CloseHandle for the thread failed\n"); + ok(CloseHandle(pipewrite), "CloseHandle for the write pipe failed\n"); + ok(CloseHandle(piperead), "CloseHandle for the read pipe failed\n"); + + /* CreateNamedPipe with PIPE_NOWAIT, test ConnectNamedPipe */ + pipewrite = CreateNamedPipeA(PIPENAME, PIPE_ACCESS_DUPLEX, PIPE_TYPE_BYTE | PIPE_NOWAIT, + /* nMaxInstances */ 1, + /* nOutBufSize */ 512, + /* nInBufSize */ 512, + /* nDefaultWait */ NMPWAIT_USE_DEFAULT_WAIT, + /* lpSecurityAttrib */ NULL); + ok(pipewrite != INVALID_HANDLE_VALUE, "CreateNamedPipe failed\n"); + /* spawn a thread that connects to the pipe after 2 seconds so the test won't hang if ConnectNamedPipe blocks */ + hThread = CreateThread(NULL, 0, test_nowait_connect_thread, NULL, 0, NULL); + ok(hThread != NULL, "CreateThread failed. %d\n", GetLastError()); + SetLastError(0xdeadbeef); + todo_wine ok(ConnectNamedPipe(pipewrite,NULL) == FALSE, "ConnectNamedPipe should fail\n"); + todo_wine ok(GetLastError() == ERROR_PIPE_LISTENING, "got %d should be ERROR_PIPE_LISTENING\n", GetLastError()); + /* wait for thread to finish (connects and disconnects), then test ConnectNamedPipe again */ + ok(WaitForSingleObject(hThread,INFINITE) == WAIT_OBJECT_0, "WaitForSingleObject\n"); + ok(CloseHandle(hThread), "CloseHandle for the thread failed\n"); + SetLastError(0xdeadbeef); + todo_wine ok(ConnectNamedPipe(pipewrite,NULL) == FALSE, "ConnectNamedPipe should fail\n"); + todo_wine ok(GetLastError() == ERROR_NO_DATA, "got %d should be ERROR_NO_DATA\n", GetLastError()); + /* call DisconnectNamedPipe and test ConnectNamedPipe again */ + ok(DisconnectNamedPipe(pipewrite) == TRUE, "DisconnectNamedPipe should succeed\n"); + hThread = CreateThread(NULL, 0, test_nowait_connect_thread, NULL, 0, NULL); + ok(hThread != NULL, "CreateThread failed. %d\n", GetLastError()); + SetLastError(0xdeadbeef); + todo_wine ok(ConnectNamedPipe(pipewrite,NULL) == FALSE, "ConnectNamedPipe should fail\n"); + todo_wine ok(GetLastError() == ERROR_PIPE_LISTENING, "got %d should be ERROR_PIPE_LISTENING\n", GetLastError()); + ok(WaitForSingleObject(hThread,INFINITE) == WAIT_OBJECT_0, "WaitForSingleObject\n"); + ok(CloseHandle(hThread), "CloseHandle for the thread failed\n"); + ok(CloseHandle(pipewrite), "CloseHandle for the write pipe failed\n"); +} + START_TEST(pipe) { char **argv; @@ -3954,4 +4061,5 @@ START_TEST(pipe) test_namedpipe_session_id(); test_multiple_instances(); test_wait_pipe(); + test_nowait(); }
Fixes Rockstar Games Launcher hanging for some users.
Signed-off-by: Brendan Shanks bshanks@codeweavers.com --- dlls/kernel32/tests/pipe.c | 4 ++-- server/named_pipe.c | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/dlls/kernel32/tests/pipe.c b/dlls/kernel32/tests/pipe.c index 227575ece3..7f2a6adca1 100644 --- a/dlls/kernel32/tests/pipe.c +++ b/dlls/kernel32/tests/pipe.c @@ -3937,8 +3937,8 @@ static void test_nowait(void) hThread = CreateThread(NULL, 0, test_nowait_read_thread, &pipewrite, 0, NULL); ok(hThread != NULL, "CreateThread failed. %d\n", GetLastError()); SetLastError(0xdeadbeef); - todo_wine ok(ReadFile(piperead,readbuf,sizeof(readbuf),&read, NULL) == FALSE, "ReadFile should fail\n"); - todo_wine ok(GetLastError() == ERROR_NO_DATA, "got %d should be ERROR_NO_DATA\n", GetLastError()); + ok(ReadFile(piperead,readbuf,sizeof(readbuf),&read, NULL) == FALSE, "ReadFile should fail\n"); + ok(GetLastError() == ERROR_NO_DATA, "got %d should be ERROR_NO_DATA\n", GetLastError()); ok(WaitForSingleObject(hThread,INFINITE) == WAIT_OBJECT_0, "WaitForSingleObject\n"); ok(CloseHandle(hThread), "CloseHandle for the thread failed\n"); ok(CloseHandle(pipewrite), "CloseHandle for the write pipe failed\n"); diff --git a/server/named_pipe.c b/server/named_pipe.c index 8e0380d060..6926712b8c 100644 --- a/server/named_pipe.c +++ b/server/named_pipe.c @@ -851,6 +851,11 @@ static int pipe_end_read( struct fd *fd, struct async *async, file_pos_t pos ) switch (pipe_end->state) { case FILE_PIPE_CONNECTED_STATE: + if ((pipe_end->flags & NAMED_PIPE_NONBLOCKING_MODE) && list_empty( &pipe_end->message_queue )) + { + set_error( STATUS_PIPE_EMPTY ); + return 0; + } break; case FILE_PIPE_DISCONNECTED_STATE: set_error( STATUS_PIPE_DISCONNECTED );
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=57232
Your paranoid android.
=== debian10 (32 bit report) ===
kernel32: pipe.c:3983: Test succeeded inside todo block: ConnectNamedPipe should fail pipe.c:3984: Test succeeded inside todo block: got 232 should be ERROR_NO_DATA
=== debian10 (32 bit French report) ===
kernel32: pipe.c:3983: Test succeeded inside todo block: ConnectNamedPipe should fail pipe.c:3984: Test succeeded inside todo block: got 232 should be ERROR_NO_DATA
=== debian10 (32 bit Japanese:Japan report) ===
kernel32: pipe.c:3983: Test succeeded inside todo block: ConnectNamedPipe should fail pipe.c:3984: Test succeeded inside todo block: got 232 should be ERROR_NO_DATA
=== debian10 (32 bit Chinese:China report) ===
kernel32: pipe.c:3983: Test succeeded inside todo block: ConnectNamedPipe should fail pipe.c:3984: Test succeeded inside todo block: got 232 should be ERROR_NO_DATA
=== debian10 (32 bit WoW report) ===
kernel32: pipe.c:3983: Test succeeded inside todo block: ConnectNamedPipe should fail pipe.c:3984: Test succeeded inside todo block: got 232 should be ERROR_NO_DATA
=== debian10 (64 bit WoW report) ===
kernel32: pipe.c:3983: Test succeeded inside todo block: ConnectNamedPipe should fail pipe.c:3984: Test succeeded inside todo block: got 232 should be ERROR_NO_DATA
Signed-off-by: Brendan Shanks bshanks@codeweavers.com --- dlls/kernel32/tests/pipe.c | 4 ++-- server/named_pipe.c | 12 +++++++----- 2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/dlls/kernel32/tests/pipe.c b/dlls/kernel32/tests/pipe.c index 7f2a6adca1..d5e516fe7d 100644 --- a/dlls/kernel32/tests/pipe.c +++ b/dlls/kernel32/tests/pipe.c @@ -3956,8 +3956,8 @@ static void test_nowait(void) /* WriteFile only documents that 'write < sizeof(readbuf)' for this case, but Windows * doesn't seem to do partial writes ('write == 0' always) */ - todo_wine ok(write < sizeof(readbuf), "WriteFile should fail to write the whole buffer\n"); - todo_wine ok(write == 0, "WriteFile shouldn't do partial writes\n"); + ok(write < sizeof(readbuf), "WriteFile should fail to write the whole buffer\n"); + ok(write == 0, "WriteFile shouldn't do partial writes\n"); ok(CloseHandle(hThread), "CloseHandle for the thread failed\n"); ok(CloseHandle(pipewrite), "CloseHandle for the write pipe failed\n"); ok(CloseHandle(piperead), "CloseHandle for the read pipe failed\n"); diff --git a/server/named_pipe.c b/server/named_pipe.c index 6926712b8c..d5716f1a1a 100644 --- a/server/named_pipe.c +++ b/server/named_pipe.c @@ -363,7 +363,7 @@ static struct pipe_message *queue_message( struct pipe_end *pipe_end, struct ios return message; }
-static void wake_message( struct pipe_message *message ) +static void wake_message( struct pipe_message *message, data_size_t result ) { struct async *async = message->async;
@@ -371,7 +371,7 @@ static void wake_message( struct pipe_message *message ) if (!async) return;
message->iosb->status = STATUS_SUCCESS; - message->iosb->result = message->iosb->in_size; + message->iosb->result = result; async_terminate( async, message->iosb->result ? STATUS_ALERTED : STATUS_SUCCESS ); release_object( async ); } @@ -749,7 +749,7 @@ static void message_queue_read( struct pipe_end *pipe_end, struct iosb *iosb ) { iosb->out_data = message->iosb->in_data; message->iosb->in_data = NULL; - wake_message( message ); + wake_message( message, message->iosb->in_size ); free_message( message ); } else @@ -773,7 +773,7 @@ static void message_queue_read( struct pipe_end *pipe_end, struct iosb *iosb ) message->read_pos += writing; if (message->read_pos == message->iosb->in_size) { - wake_message(message); + wake_message(message, message->iosb->in_size); free_message(message); } } while (write_pos < iosb->out_size); @@ -836,7 +836,9 @@ static void reselect_write_queue( struct pipe_end *pipe_end ) { avail += message->iosb->in_size - message->read_pos; if (message->async && (avail <= reader->buffer_size || !message->iosb->in_size)) - wake_message( message ); + wake_message( message, message->iosb->in_size ); + else if (message->async && (pipe_end->flags & NAMED_PIPE_NONBLOCKING_MODE)) + wake_message( message, 0 ); } }
On 10/1/19 6:16 PM, Brendan Shanks wrote:
wake_message( message, message->iosb->in_size );else if (message->async && (pipe_end->flags & NAMED_PIPE_NONBLOCKING_MODE))wake_message( message, 0 );
Note that this will terminate write async, but leave the message queued. Is it intended to do that?
Jacek
On Oct 1, 2019, at 9:51 AM, Jacek Caban jacek@codeweavers.com wrote:
On 10/1/19 6:16 PM, Brendan Shanks wrote:
wake_message( message, message->iosb->in_size );else if (message->async && (pipe_end->flags & NAMED_PIPE_NONBLOCKING_MODE))wake_message( message, 0 );Note that this will terminate write async, but leave the message queued. Is it intended to do that?
No it’s not, thanks for catching that. Would I just need to call free_message() after wake_message()?
Brendan
On 10/1/19 8:21 PM, Brendan Shanks wrote:
On Oct 1, 2019, at 9:51 AM, Jacek Caban jacek@codeweavers.com wrote:
On 10/1/19 6:16 PM, Brendan Shanks wrote:
wake_message( message, message->iosb->in_size );else if (message->async && (pipe_end->flags & NAMED_PIPE_NONBLOCKING_MODE))wake_message( message, 0 );Note that this will terminate write async, but leave the message queued. Is it intended to do that?
No it’s not, thanks for catching that. Would I just need to call free_message() after wake_message()?
Note that there is one more problem with this: there may be a pending read, which current write may satisfy and thus complete without blocking. It would really need tests to be sure, but I'd expect that we should check if the call will block before queuing it. It may be useful to keep track of total pending read quota in pipe end, but that's just a guess without tests.
Jacek
On Oct 1, 2019, at 11:38 AM, Jacek Caban jacek@codeweavers.com wrote:
On 10/1/19 8:21 PM, Brendan Shanks wrote:
On Oct 1, 2019, at 9:51 AM, Jacek Caban jacek@codeweavers.com wrote:
On 10/1/19 6:16 PM, Brendan Shanks wrote:
wake_message( message, message->iosb->in_size );else if (message->async && (pipe_end->flags & NAMED_PIPE_NONBLOCKING_MODE))wake_message( message, 0 );Note that this will terminate write async, but leave the message queued. Is it intended to do that?
No it’s not, thanks for catching that. Would I just need to call free_message() after wake_message()?
Note that there is one more problem with this: there may be a pending read, which current write may satisfy and thus complete without blocking. It would really need tests to be sure, but I'd expect that we should check if the call will block before queuing it. It may be useful to keep track of total pending read quota in pipe end, but that's just a guess without tests.
Hi Jacek,
In v2 of the patch I added a test for this (an overlapped read followed by a nonblocking write larger than the pipe buffer), on Wine the write was returning with 0 bytes written. My fix was to only do the wake/free from reselect_write_queue() if there’s no asyncs waiting in the reader read_q, not sure if this is correct or I’m overlooking something though.
Brendan
Hi Brendan,
On 10/3/19 12:45 AM, Brendan Shanks wrote:
Hi Jacek, In v2 of the patch I added a test for this (an overlapped read followed by a nonblocking write larger than the pipe buffer), on Wine the write was returning with 0 bytes written. My fix was to only do the wake/free from reselect_write_queue() if there’s no asyncs waiting in the reader read_q, not sure if this is correct or I’m overlooking something though.
Tests looks much better now, thanks. Implementation seems questionable because non-blocking write may block now. For an example do a pending read with size 1 followed by write of size 514. I'd expect write to fail and read to not be satisfied on Windows. I think that with your implementation, read will be satisfied and write will block.
Jacek
On Oct 3, 2019, at 5:01 AM, Jacek Caban jacek@codeweavers.com wrote:
Hi Brendan,
On 10/3/19 12:45 AM, Brendan Shanks wrote:
Hi Jacek, In v2 of the patch I added a test for this (an overlapped read followed by a nonblocking write larger than the pipe buffer), on Wine the write was returning with 0 bytes written. My fix was to only do the wake/free from reselect_write_queue() if there’s no asyncs waiting in the reader read_q, not sure if this is correct or I’m overlooking something though.
Tests looks much better now, thanks. Implementation seems questionable because non-blocking write may block now. For an example do a pending read with size 1 followed by write of size 514. I'd expect write to fail and read to not be satisfied on Windows. I think that with your implementation, read will be satisfied and write will block.
I tried this out, on Windows the write succeeds but only writes 1 byte, and the read completes with 1 byte as well. On Wine this almost works right, but WriteFile returns 0 for the bytes written even though the read completes with 1 byte. I’ll fix this and add these tests for v3.
I also tried a pending read of 1 byte and write 513 bytes, in that case the read and write both complete fully and the 512 bytes stays in the pipe for the next read call.
Brendan
On 10/3/19 8:11 PM, Brendan Shanks wrote:
On Oct 3, 2019, at 5:01 AM, Jacek Caban jacek@codeweavers.com wrote:
Hi Brendan,
On 10/3/19 12:45 AM, Brendan Shanks wrote:
Hi Jacek, In v2 of the patch I added a test for this (an overlapped read followed by a nonblocking write larger than the pipe buffer), on Wine the write was returning with 0 bytes written. My fix was to only do the wake/free from reselect_write_queue() if there’s no asyncs waiting in the reader read_q, not sure if this is correct or I’m overlooking something though.
Tests looks much better now, thanks. Implementation seems questionable because non-blocking write may block now. For an example do a pending read with size 1 followed by write of size 514. I'd expect write to fail and read to not be satisfied on Windows. I think that with your implementation, read will be satisfied and write will block.
I tried this out, on Windows the write succeeds but only writes 1 byte, and the read completes with 1 byte as well. On Wine this almost works right, but WriteFile returns 0 for the bytes written even though the read completes with 1 byte. I’ll fix this and add these tests for v3.
Okay, so Windows does partial writes after all. I will comment on the new patch separately, but it looks almost good now.
I also tried a pending read of 1 byte and write 513 bytes, in that case the read and write both complete fully and the 512 bytes stays in the pipe for the next read call.
FWIW, it's not really needed in this series, but another interesting corner case would be a pending 0-size read in message mode and write of 513 bytes. I don't expect surprises through.
Thanks,
Jacek
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=57233
Your paranoid android.
=== debian10 (32 bit report) ===
kernel32: pipe.c:3983: Test succeeded inside todo block: ConnectNamedPipe should fail pipe.c:3984: Test succeeded inside todo block: got 232 should be ERROR_NO_DATA
=== debian10 (32 bit French report) ===
kernel32: pipe.c:3983: Test succeeded inside todo block: ConnectNamedPipe should fail pipe.c:3984: Test succeeded inside todo block: got 232 should be ERROR_NO_DATA
=== debian10 (32 bit Japanese:Japan report) ===
kernel32: pipe.c:3983: Test succeeded inside todo block: ConnectNamedPipe should fail pipe.c:3984: Test succeeded inside todo block: got 232 should be ERROR_NO_DATA
=== debian10 (32 bit Chinese:China report) ===
kernel32: pipe.c:3983: Test succeeded inside todo block: ConnectNamedPipe should fail pipe.c:3984: Test succeeded inside todo block: got 232 should be ERROR_NO_DATA
=== debian10 (32 bit WoW report) ===
kernel32: pipe.c:3983: Test succeeded inside todo block: ConnectNamedPipe should fail pipe.c:3984: Test succeeded inside todo block: got 232 should be ERROR_NO_DATA
=== debian10 (64 bit WoW report) ===
kernel32: pipe.c:3983: Test succeeded inside todo block: ConnectNamedPipe should fail pipe.c:3984: Test succeeded inside todo block: got 232 should be ERROR_NO_DATA
Signed-off-by: Brendan Shanks bshanks@codeweavers.com --- dlls/kernel32/tests/pipe.c | 12 ++++++------ server/named_pipe.c | 6 ++++++ 2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/dlls/kernel32/tests/pipe.c b/dlls/kernel32/tests/pipe.c index d5e516fe7d..e018c557bc 100644 --- a/dlls/kernel32/tests/pipe.c +++ b/dlls/kernel32/tests/pipe.c @@ -3974,21 +3974,21 @@ static void test_nowait(void) hThread = CreateThread(NULL, 0, test_nowait_connect_thread, NULL, 0, NULL); ok(hThread != NULL, "CreateThread failed. %d\n", GetLastError()); SetLastError(0xdeadbeef); - todo_wine ok(ConnectNamedPipe(pipewrite,NULL) == FALSE, "ConnectNamedPipe should fail\n"); - todo_wine ok(GetLastError() == ERROR_PIPE_LISTENING, "got %d should be ERROR_PIPE_LISTENING\n", GetLastError()); + ok(ConnectNamedPipe(pipewrite,NULL) == FALSE, "ConnectNamedPipe should fail\n"); + ok(GetLastError() == ERROR_PIPE_LISTENING, "got %d should be ERROR_PIPE_LISTENING\n", GetLastError()); /* wait for thread to finish (connects and disconnects), then test ConnectNamedPipe again */ ok(WaitForSingleObject(hThread,INFINITE) == WAIT_OBJECT_0, "WaitForSingleObject\n"); ok(CloseHandle(hThread), "CloseHandle for the thread failed\n"); SetLastError(0xdeadbeef); - todo_wine ok(ConnectNamedPipe(pipewrite,NULL) == FALSE, "ConnectNamedPipe should fail\n"); - todo_wine ok(GetLastError() == ERROR_NO_DATA, "got %d should be ERROR_NO_DATA\n", GetLastError()); + ok(ConnectNamedPipe(pipewrite,NULL) == FALSE, "ConnectNamedPipe should fail\n"); + ok(GetLastError() == ERROR_NO_DATA, "got %d should be ERROR_NO_DATA\n", GetLastError()); /* call DisconnectNamedPipe and test ConnectNamedPipe again */ ok(DisconnectNamedPipe(pipewrite) == TRUE, "DisconnectNamedPipe should succeed\n"); hThread = CreateThread(NULL, 0, test_nowait_connect_thread, NULL, 0, NULL); ok(hThread != NULL, "CreateThread failed. %d\n", GetLastError()); SetLastError(0xdeadbeef); - todo_wine ok(ConnectNamedPipe(pipewrite,NULL) == FALSE, "ConnectNamedPipe should fail\n"); - todo_wine ok(GetLastError() == ERROR_PIPE_LISTENING, "got %d should be ERROR_PIPE_LISTENING\n", GetLastError()); + ok(ConnectNamedPipe(pipewrite,NULL) == FALSE, "ConnectNamedPipe should fail\n"); + ok(GetLastError() == ERROR_PIPE_LISTENING, "got %d should be ERROR_PIPE_LISTENING\n", GetLastError()); ok(WaitForSingleObject(hThread,INFINITE) == WAIT_OBJECT_0, "WaitForSingleObject\n"); ok(CloseHandle(hThread), "CloseHandle for the thread failed\n"); ok(CloseHandle(pipewrite), "CloseHandle for the write pipe failed\n"); diff --git a/server/named_pipe.c b/server/named_pipe.c index d5716f1a1a..e4f7a89aa3 100644 --- a/server/named_pipe.c +++ b/server/named_pipe.c @@ -1102,6 +1102,12 @@ static int pipe_server_ioctl( struct fd *fd, ioctl_code_t code, struct async *as return 0; }
+ if (server->pipe_end.flags & NAMED_PIPE_NONBLOCKING_MODE) + { + set_error( STATUS_PIPE_LISTENING ); + return 0; + } + queue_async( &server->listen_q, async ); async_wake_up( &server->pipe_end.pipe->waiters, STATUS_SUCCESS ); set_error( STATUS_PENDING );
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=57234
Your paranoid android.
=== w8 (32 bit report) ===
kernel32: pipe.c:2855: Test failed: FlushFileBuffers failed: 87
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=57231
Your paranoid android.
=== debian10 (32 bit report) ===
kernel32: pipe.c:3983: Test succeeded inside todo block: ConnectNamedPipe should fail pipe.c:3984: Test succeeded inside todo block: got 232 should be ERROR_NO_DATA
=== debian10 (32 bit French report) ===
kernel32: pipe.c:3983: Test succeeded inside todo block: ConnectNamedPipe should fail pipe.c:3984: Test succeeded inside todo block: got 232 should be ERROR_NO_DATA
=== debian10 (32 bit Japanese:Japan report) ===
kernel32: pipe.c:3983: Test succeeded inside todo block: ConnectNamedPipe should fail pipe.c:3984: Test succeeded inside todo block: got 232 should be ERROR_NO_DATA
=== debian10 (32 bit Chinese:China report) ===
kernel32: pipe.c:3983: Test succeeded inside todo block: ConnectNamedPipe should fail pipe.c:3984: Test succeeded inside todo block: got 232 should be ERROR_NO_DATA
=== debian10 (32 bit WoW report) ===
kernel32: pipe.c:3983: Test succeeded inside todo block: ConnectNamedPipe should fail pipe.c:3984: Test succeeded inside todo block: got 232 should be ERROR_NO_DATA
=== debian10 (64 bit WoW report) ===
kernel32: pipe.c:3983: Test succeeded inside todo block: ConnectNamedPipe should fail pipe.c:3984: Test succeeded inside todo block: got 232 should be ERROR_NO_DATA
Hi Brendan,
On 10/1/19 6:16 PM, Brendan Shanks wrote:
Signed-off-by: Brendan Shanks bshanks@codeweavers.com
dlls/kernel32/tests/pipe.c | 108 +++++++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+)
diff --git a/dlls/kernel32/tests/pipe.c b/dlls/kernel32/tests/pipe.c index f61d441303..227575ece3 100644 --- a/dlls/kernel32/tests/pipe.c +++ b/dlls/kernel32/tests/pipe.c @@ -3887,6 +3887,113 @@ static void test_wait_pipe(void) CloseHandle(ov.hEvent); }
+static DWORD CALLBACK test_nowait_read_thread(LPVOID arg) +{
- HANDLE *pipewrite = arg;
- static const char buf[] = "bits";
- DWORD written;
- Sleep(2000);
- WriteFile(*pipewrite, buf, sizeof(buf), &written, NULL);
- return 0;
+} +static DWORD CALLBACK test_nowait_write_thread(LPVOID arg) +{
- HANDLE *piperead = arg;
- char buf[32768];
- DWORD read;
- Sleep(2000);
- ReadFile(*piperead, buf, sizeof(buf), &read, NULL);
- return 0;
+} +static DWORD CALLBACK test_nowait_connect_thread(LPVOID arg) +{
- HANDLE hFile;
- Sleep(2000);
- hFile = CreateFileA(PIPENAME, GENERIC_READ | GENERIC_WRITE, 0, NULL, OPEN_EXISTING, 0, 0);
- ok(hFile != INVALID_HANDLE_VALUE, "CreateFileA failed\n");
- ok(CloseHandle(hFile), "CloseHandle failed\n");
- return 0;
+}
I'd expect that those tests would be more reliable and cleaner if you used overlapped handles. You could have one pipe end non-blocking and the other end overlapped to test everything using one thread in a predictable way. If you carefully choose buffer sizes, you could make those tests more precise. See test_blocking_rw for an example.
- /* WriteFile only documents that 'write < sizeof(readbuf)' for this case, but Windows
* doesn't seem to do partial writes ('write == 0' always)*/
If Windows doesn't do partial writes, it would be actually interesting to test and implement.
Thanks,
Jacek
-
Brendan Shanks -
Jacek Caban -
Marvin