Stefan Dösinger : ntdll: Make RtlDeregisterWaitEx( handle, INVALID_HANDLE_VALUE) thread safe.

[Alexandre Julliard]

Module: wine
Branch: master
Commit: af35aada9b078f8dc71dcc85e505da8eee4571da
URL:    http://source.winehq.org/git/wine.git/?a=commit;h=af35aada9b078f8dc71dcc85e5...

Author: Stefan Dösinger <stefan(a)codeweavers.com>
Date:   Tue Sep  5 17:34:03 2017 +0200

ntdll: Make RtlDeregisterWaitEx(handle, INVALID_HANDLE_VALUE) thread safe.

Chromium signals the wait semaphore and calls DeregisterWaitEx with
CompletionHandle = INVALID_HANDLE_VALUE in close succession. Sometimes
the worker thread decides to run the callback, but before it sets
CallbackInProgress RtlDeregisterWaitEx decides that the callback is not
running and returns STATUS_SUCCESS. Chromium then releases resources
that the callback needs to run, resulting in random crashes.

Signed-off-by: Stefan Dösinger <stefan(a)codeweavers.com>
Signed-off-by: Sebastian Lackner <sebastian(a)fds-team.de>
Signed-off-by: Alexandre Julliard <julliard(a)winehq.org>

---

 dlls/ntdll/threadpool.c | 59 +++++++++++++++++++++++++++----------------------
 1 file changed, 32 insertions(+), 27 deletions(-)

diff --git a/dlls/ntdll/threadpool.c b/dlls/ntdll/threadpool.c
index 6063d51..1b0546a 100644
--- a/dlls/ntdll/threadpool.c
+++ b/dlls/ntdll/threadpool.c
@@ -541,11 +541,13 @@ static DWORD CALLBACK wait_thread_proc(LPVOID Arg)
             break;
     }
 
-    completion_event = wait_work_item->CompletionEvent;
-    if (completion_event) NtSetEvent( completion_event, NULL );
 
     if (interlocked_inc( &wait_work_item->DeleteCount ) == 2 )
+    {
+        completion_event = wait_work_item->CompletionEvent;
         delete_wait_work_item( wait_work_item );
+        if (completion_event) NtSetEvent( completion_event, NULL );
+    }
 
     return 0;
 }
@@ -633,44 +635,47 @@ NTSTATUS WINAPI RtlRegisterWait(PHANDLE NewWaitObject, HANDLE Object,
 NTSTATUS WINAPI RtlDeregisterWaitEx(HANDLE WaitHandle, HANDLE CompletionEvent)
 {
     struct wait_work_item *wait_work_item = WaitHandle;
-    NTSTATUS status = STATUS_SUCCESS;
+    NTSTATUS status;
+    HANDLE LocalEvent = NULL;
+    BOOLEAN CallbackInProgress;
 
-    TRACE( "(%p)\n", WaitHandle );
+    TRACE( "(%p %p)\n", WaitHandle, CompletionEvent );
 
     if (WaitHandle == NULL)
         return STATUS_INVALID_HANDLE;
 
-    NtSetEvent( wait_work_item->CancelEvent, NULL );
-    if (wait_work_item->CallbackInProgress)
+    CallbackInProgress = wait_work_item->CallbackInProgress;
+    if (CompletionEvent == INVALID_HANDLE_VALUE || !CallbackInProgress)
     {
-        if (CompletionEvent != NULL)
-        {
-            if (CompletionEvent == INVALID_HANDLE_VALUE)
-            {
-                status = NtCreateEvent( &CompletionEvent, EVENT_ALL_ACCESS, NULL, NotificationEvent, FALSE );
-                if (status != STATUS_SUCCESS)
-                    return status;
-                interlocked_xchg_ptr( &wait_work_item->CompletionEvent, CompletionEvent );
-                if (wait_work_item->CallbackInProgress)
-                    NtWaitForSingleObject( CompletionEvent, FALSE, NULL );
-                NtClose( CompletionEvent );
-            }
-            else
-            {
-                interlocked_xchg_ptr( &wait_work_item->CompletionEvent, CompletionEvent );
-                if (wait_work_item->CallbackInProgress)
-                    status = STATUS_PENDING;
-            }
-        }
-        else
-            status = STATUS_PENDING;
+        status = NtCreateEvent( &LocalEvent, EVENT_ALL_ACCESS, NULL, NotificationEvent, FALSE );
+        if (status != STATUS_SUCCESS)
+            return status;
+        interlocked_xchg_ptr( &wait_work_item->CompletionEvent, LocalEvent );
+    }
+    else if (CompletionEvent != NULL)
+    {
+        interlocked_xchg_ptr( &wait_work_item->CompletionEvent, CompletionEvent );
     }
 
+    NtSetEvent( wait_work_item->CancelEvent, NULL );
+
     if (interlocked_inc( &wait_work_item->DeleteCount ) == 2 )
     {
         status = STATUS_SUCCESS;
         delete_wait_work_item( wait_work_item );
     }
+    else if (LocalEvent)
+    {
+        NtWaitForSingleObject( LocalEvent, FALSE, NULL );
+        status = STATUS_SUCCESS;
+    }
+    else
+    {
+        status = STATUS_PENDING;
+    }
+
+    if (LocalEvent)
+        NtClose( LocalEvent );
 
     return status;
 }