[PATCH 1/3] server: Abort waiting on a completion port when closing it.

[Alexey Prokhin]

Signed-off-by: Alexey Prokhin <alexey(a)prokhin.ru>
---
 server/completion.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/server/completion.c b/server/completion.c
index db04727b93..9e15c1ca07 100644
--- a/server/completion.c
+++ b/server/completion.c
@@ -48,12 +48,14 @@ struct completion
     struct object  obj;
     struct list    queue;
     unsigned int   depth;
+    int            abandoned;
 };
 
 static void completion_dump( struct object*, int );
 static struct object_type *completion_get_type( struct object *obj );
 static int completion_signaled( struct object *obj, struct wait_queue_entry *entry );
 static unsigned int completion_map_access( struct object *obj, unsigned int access );
+static int completion_close( struct object *obj, struct process *process, obj_handle_t handle );
 static void completion_destroy( struct object * );
 
 static const struct object_ops completion_ops =
@@ -75,7 +77,7 @@ static const struct object_ops completion_ops =
     default_unlink_name,       /* unlink_name */
     no_open_file,              /* open_file */
     no_kernel_obj_list,        /* get_kernel_obj_list */
-    no_close_handle,           /* close_handle */
+    completion_close,          /* close_handle */
     completion_destroy         /* destroy */
 };
 
@@ -88,6 +90,22 @@ struct comp_msg
     unsigned int  status;
 };
 
+static int completion_close( struct object *obj, struct process *process, obj_handle_t handle )
+{
+    struct completion *completion = (struct completion *) obj;
+    struct wait_queue_entry *entry;
+
+    LIST_FOR_EACH_ENTRY( entry, &obj->wait_queue, struct wait_queue_entry, entry )
+    {
+        make_wait_abandoned( entry );
+    }
+
+    completion->abandoned = 1;
+    wake_up( &completion->obj, 0 );
+
+    return 1;
+}
+
 static void completion_destroy( struct object *obj)
 {
     struct completion *completion = (struct completion *) obj;
@@ -118,7 +136,7 @@ static int completion_signaled( struct object *obj, struct wait_queue_entry *ent
 {
     struct completion *completion = (struct completion *)obj;
 
-    return !list_empty( &completion->queue );
+    return !list_empty( &completion->queue ) || completion->abandoned;
 }
 
 static unsigned int completion_map_access( struct object *obj, unsigned int access )
@@ -141,6 +159,7 @@ static struct completion *create_completion( struct object *root, const struct u
         if (get_error() != STATUS_OBJECT_NAME_EXISTS)
         {
             list_init( &completion->queue );
+            completion->abandoned = 0;
             completion->depth = 0;
         }
     }
@@ -224,7 +243,7 @@ DECL_HANDLER(remove_completion)
 
     entry = list_head( &completion->queue );
     if (!entry)
-        set_error( STATUS_PENDING );
+        set_error( completion->abandoned ? STATUS_ABANDONED_WAIT_0 : STATUS_PENDING );
     else
     {
         list_remove( entry );
-- 
2.26.2

[PATCH 2/3] kernelbase: Set the proper error code in GetQueuedCompletionStatus{Ex} when the handle is closed.

[Alexey Prokhin]

Planet Zoo relies on it being ERROR_ABANDONED_WAIT_0.

Signed-off-by: Alexey Prokhin <alexey(a)prokhin.ru>
---
 dlls/kernelbase/sync.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/dlls/kernelbase/sync.c b/dlls/kernelbase/sync.c
index c0db4679e9..82f63a051c 100644
--- a/dlls/kernelbase/sync.c
+++ b/dlls/kernelbase/sync.c
@@ -959,6 +959,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetQueuedCompletionStatus( HANDLE port, LPDWORD co
     }
 
     if (status == STATUS_TIMEOUT) SetLastError( WAIT_TIMEOUT );
+    else if (status == ERROR_WAIT_NO_CHILDREN) SetLastError( ERROR_ABANDONED_WAIT_0 );
     else SetLastError( RtlNtStatusToDosError(status) );
     return FALSE;
 }
@@ -980,6 +981,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetQueuedCompletionStatusEx( HANDLE port, OVERLAPP
     if (ret == STATUS_SUCCESS) return TRUE;
     else if (ret == STATUS_TIMEOUT) SetLastError( WAIT_TIMEOUT );
     else if (ret == STATUS_USER_APC) SetLastError( WAIT_IO_COMPLETION );
+    else if (ret == ERROR_WAIT_NO_CHILDREN) SetLastError( ERROR_ABANDONED_WAIT_0 );
     else SetLastError( RtlNtStatusToDosError(ret) );
     return FALSE;
 }
-- 
2.26.2

[PATCH 2/3] kernelbase: Set the proper error code in GetQueuedCompletionStatus{Ex} when the handle is closed.

[Dmitry Timoshkov]

Alexey Prokhin <alexey(a)prokhin.ru> wrote:
> if (status == STATUS_TIMEOUT) SetLastError( WAIT_TIMEOUT );
> +    else if (status == ERROR_WAIT_NO_CHILDREN) SetLastError( ERROR_ABANDONED_WAIT_0 );
>      else SetLastError( RtlNtStatusToDosError(status) );
>      return FALSE;
>  }
> @@ -980,6 +981,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetQueuedCompletionStatusEx( HANDLE port, OVERLAPP
>      if (ret == STATUS_SUCCESS) return TRUE;
>      else if (ret == STATUS_TIMEOUT) SetLastError( WAIT_TIMEOUT );
>      else if (ret == STATUS_USER_APC) SetLastError( WAIT_IO_COMPLETION );
> +    else if (ret == ERROR_WAIT_NO_CHILDREN) SetLastError( ERROR_ABANDONED_WAIT_0 );
>      else SetLastError( RtlNtStatusToDosError(ret) );
>      return FALSE;
>  }

Probably 'ret' should be checked for appropriate STATUS_xxx code, accidently
ERROR_WAIT_NO_CHILDREN is equal STATUS_ABANDONED_WAIT_0.

-- 
Dmitry.

[PATCH 3/3] ntdll/tests: Add a test for closing a completion port being waited on.

[Alexey Prokhin]

The test reproduces a hang by Planet Zoo when exiting the game.

Signed-off-by: Alexey Prokhin <alexey(a)prokhin.ru>

---

Given it is one of my first contributions to wine, I have a few
questions/notes:

1.
> LPTHREAD_START_ROUTINE test_routines[] = {

I didn't find such pattern anywhere else in the tests, but I hope
it's ok.

2.
> ok( info[0].CompletionKey == 0xdeadbeef || info[0].CompletionKey == 0,

I am not sure if info should be checked at all in the case. And if
it is, should I mark the second equality as broken? (Because info
is only zeroed out on win server 2008)

3. Is it ok there is a test for GetQueuedCompletionStatus. Or
should it be moved to kernelbase?

4. Should a test for GetQueuedCompletionStatusEx also be added?
---
 dlls/ntdll/tests/file.c | 101 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 101 insertions(+)

diff --git a/dlls/ntdll/tests/file.c b/dlls/ntdll/tests/file.c
index 31c18454f0..c5fb28e16f 100644
--- a/dlls/ntdll/tests/file.c
+++ b/dlls/ntdll/tests/file.c
@@ -4953,6 +4953,106 @@ static void test_file_readonly_access(void)
     DeleteFileW(path);
 }
 
+static DWORD WINAPI io_thread_NtRemoveIoCompletion(void *data)
+{
+    LARGE_INTEGER timeout;
+    IO_STATUS_BLOCK iosb;
+    ULONG_PTR key, value;
+    HANDLE h = data;
+    NTSTATUS res;
+
+    timeout.QuadPart = -30000000;
+    res = pNtRemoveIoCompletion( h, &key, &value, &iosb, &timeout );
+    ok( res == STATUS_ABANDONED_WAIT_0 || broken( res == STATUS_TIMEOUT ), /* XP/2003 */
+        "expected STATUS_ABANDONED_WAIT_0, got %#x\n", res );
+    /* key, value and iosb either are unchanged or contain garbage */
+
+    return 0;
+}
+
+static DWORD WINAPI io_thread_NtRemoveIoCompletionEx(void *data)
+{
+    FILE_IO_COMPLETION_INFORMATION info[2] = {{0}};
+    LARGE_INTEGER timeout;
+    HANDLE h = data;
+    NTSTATUS res;
+    ULONG count;
+
+    if (!pNtRemoveIoCompletionEx)
+    {
+        skip("NtRemoveIoCompletionEx() not present\n");
+        return 1;
+    }
+
+    timeout.QuadPart = -30000000;
+    count = 0xdeadbeef;
+    info[0].CompletionKey = 0xdeadbeef;
+    info[0].CompletionValue = 0xdeadbeef;
+    info[0].IoStatusBlock.Information = 0xdeadbeef;
+    U(info[0].IoStatusBlock).Status = 0xdeadbeef;
+
+    res = pNtRemoveIoCompletionEx( h, info, 2, &count, &timeout, FALSE );
+    ok( res == STATUS_ABANDONED_WAIT_0, "expected STATUS_ABANDONED_WAIT_0, got %#x\n", res );
+    ok( count == 1, "wrong count %u\n", count );
+    ok( info[0].CompletionKey == 0xdeadbeef || info[0].CompletionKey == 0,
+        "wrong key %#lx\n", info[0].CompletionKey );
+    ok( info[0].CompletionValue == 0xdeadbeef || info[0].CompletionValue == 0,
+        "wrong value %#lx\n", info[0].CompletionValue );
+    ok( info[0].IoStatusBlock.Information == 0xdeadbeef || info[0].IoStatusBlock.Information == 0,
+        "wrong information %#lx\n", info[0].IoStatusBlock.Information );
+    ok( U(info[0].IoStatusBlock).Status == 0xdeadbeef || U(info[0].IoStatusBlock).Status == 0,
+        "wrong status %#x\n", U(info[0].IoStatusBlock).Status);
+
+    return 0;
+}
+
+static DWORD WINAPI io_thread_GetQueuedCompletionStatus(void *data)
+{
+    HANDLE h = data;
+    OVERLAPPED *pov;
+    NTSTATUS error;
+    ULONG_PTR key;
+    DWORD count;
+    BOOL ret;
+
+    pov = (void *)0xdeadbeaf;
+    SetLastError(0xdeadbeef);
+    ret = GetQueuedCompletionStatus( h, &count, &key, &pov, 3000 );
+    error = GetLastError();
+    ok( !ret, "GetQueuedCompletionStatus succeeded\n" );
+    ok( error == ERROR_ABANDONED_WAIT_0 || broken( error == WAIT_TIMEOUT ), /* XP/2003 */
+        "expected ERROR_ABANDONED_WAIT_0, got %#x\n", error );
+    ok( pov == NULL, "expected NULL, got %p\n", pov );
+
+    return 0;
+}
+
+static void test_close_io_completion(void)
+{
+    HANDLE h, hThread;
+    NTSTATUS res;
+    LPTHREAD_START_ROUTINE test_routines[] = {
+        io_thread_NtRemoveIoCompletion,
+        io_thread_NtRemoveIoCompletionEx,
+        io_thread_GetQueuedCompletionStatus
+    };
+
+    for (int i = 0; i < ARRAY_SIZE(test_routines); ++i)
+    {
+        res = pNtCreateIoCompletion( &h, IO_COMPLETION_ALL_ACCESS, NULL, 0 );
+        ok( res == STATUS_SUCCESS, "%d: NtCreateIoCompletion failed: %#x\n", i, res );
+        ok( h && h != INVALID_HANDLE_VALUE, "%d: got invalid handle %p\n", i, h );
+
+        hThread = CreateThread( NULL, 0, test_routines[i], h, 0, NULL );
+        Sleep( 400 );
+
+        CloseHandle( h );
+
+        ok( WaitForSingleObject( hThread, 10000 ) == 0, "%d: wait failed\n", i );
+        CloseHandle( hThread );
+    }
+}
+
 START_TEST(file)
 {
     HMODULE hkernel32 = GetModuleHandleA("kernel32.dll");
@@ -5023,4 +5123,5 @@ START_TEST(file)
     test_query_attribute_information_file();
     test_ioctl();
     test_flush_buffers_file();
+    test_close_io_completion();
 }
-- 
2.26.2