[PATCH] kernel32/tests: Make sure the child process is done running its tests before terminating it.

[Zebediah Figura]

Signed-off-by: Zebediah Figura 
---
 dlls/kernel32/tests/pipe.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/dlls/kernel32/tests/pipe.c b/dlls/kernel32/tests/pipe.c
index 922eed9..1708c28 100644
--- a/dlls/kernel32/tests/pipe.c
+++ b/dlls/kernel32/tests/pipe.c
@@ -2962,10 +2962,15 @@ static void child_process_write_pipe(HANDLE pipe)
 {
     OVERLAPPED overlapped;
     char buf[10000];
+    HANDLE event;

memset(buf, 'x', sizeof(buf));
     overlapped_write_async(pipe, buf, sizeof(buf), &overlapped);

+    event = OpenEventA(EVENT_MODIFY_STATE, FALSE, "wine_test_pipe_event");
+    SetEvent(event);
+    CloseHandle(event);
+
     /* sleep until parent process terminates this process */
     Sleep(INFINITE);
 }
@@ -3024,6 +3029,7 @@ static void test_overlapped_transport(BOOL msg_mode, BOOL msg_read_mode)
     HANDLE process;
     char buf[60000];
     BOOL res;
+    HANDLE event;

DWORD create_flags =
         (msg_mode ? PIPE_TYPE_MESSAGE : PIPE_TYPE_BYTE) |
@@ -3074,6 +3080,7 @@ static void test_overlapped_transport(BOOL msg_mode, BOOL msg_read_mode)

/* terminate process with pending write */
     create_overlapped_pipe(create_flags, &client, &server);
+    event = CreateEventA(NULL, FALSE, FALSE, "wine_test_pipe_event");
     process = create_writepipe_process(client);
     /* successfully read part of write that is pending in child process */
     res = ReadFile(server, buf, 10, &read_bytes, NULL);
@@ -3082,6 +3089,7 @@ static void test_overlapped_transport(BOOL msg_mode, BOOL msg_read_mode)
     else
         ok(!res && GetLastError() == ERROR_MORE_DATA, "ReadFile returned: %x %u\n", res, GetLastError());
     ok(read_bytes == 10, "read_bytes = %u\n", read_bytes);
+    ok(WaitForSingleObject(event, 1000) == WAIT_OBJECT_0, "Wait failed\n");
     TerminateProcess(process, 0);
     winetest_wait_child_process(process);
     /* after terminating process, there is no pending write and pipe buffer is empty */

[Alexandre Julliard]

Zebediah Figura  writes:
> Signed-off-by: Zebediah Figura
>
> dlls/kernel32/tests/pipe.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/dlls/kernel32/tests/pipe.c b/dlls/kernel32/tests/pipe.c
> index 922eed9..1708c28 100644
> --- a/dlls/kernel32/tests/pipe.c
> +++ b/dlls/kernel32/tests/pipe.c
> @@ -2962,10 +2962,15 @@ static void child_process_write_pipe(HANDLE pipe)
>  {
>      OVERLAPPED overlapped;
>      char buf[10000];
> -
>
> HANDLE event;
>
> memset(buf, 'x', sizeof(buf));
> overlapped_write_async(pipe, buf, sizeof(buf), &overlapped);
> -
>
> event = OpenEventA(EVENT_MODIFY_STATE, FALSE, "wine_test_pipe_event");
> -
>
> SetEvent(event);
> -
>
> CloseHandle(event);
> -

It would be more robust to pass the event to the child instead of using
a global name. Also as long as we are synchronizing them, having the
parent signal the child to exit would be cleaner than using
TerminateProcess.

[Jacek Caban]

On 01/22/2018 08:01 PM, Alexandre Julliard wrote:
> Zebediah Figura  writes:
> > Signed-off-by: Zebediah Figura
> >
> > dlls/kernel32/tests/pipe.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/dlls/kernel32/tests/pipe.c b/dlls/kernel32/tests/pipe.c
> > index 922eed9..1708c28 100644
> > --- a/dlls/kernel32/tests/pipe.c
> > +++ b/dlls/kernel32/tests/pipe.c
> > @@ -2962,10 +2962,15 @@ static void child_process_write_pipe(HANDLE pipe)
> >  {
> >      OVERLAPPED overlapped;
> >      char buf[10000];
> > -
> >
> > HANDLE event;
> >
> > memset(buf, 'x', sizeof(buf));
> > overlapped_write_async(pipe, buf, sizeof(buf), &overlapped);
> > -
> >
> > event = OpenEventA(EVENT_MODIFY_STATE, FALSE, "wine_test_pipe_event");
> > -
> >
> > SetEvent(event);
> > -
> >
> > CloseHandle(event);
> > -
>
> It would be more robust to pass the event to the child instead of using
> a global name. Also as long as we are synchronizing them, having the
> parent signal the child to exit would be cleaner than using
> TerminateProcess.

TerminateProcess here was intentional because it triggered a more
interesting code path in server, but sure, it can be changed.

The problem with this test is a todo_wine like in output. I was thinking
about fixing the problem in kernel32 shown by the test and removing
todo_wine.

Thanks,

Jacek