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@codeweavers.com> Signed-off-by: Sebastian Lackner <sebastian@fds-team.de> Signed-off-by: Alexandre Julliard <julliard@winehq.org>

ntdll: Make RtlDeregisterWaitEx(handle, INVALID_HANDLE_VALUE) thread safe.
af35aada · Stefan Dösinger · Alexandre Julliard · 41236123 · af35aada
Commit af35aada authored Sep 05, 2017 by Stefan Dösinger Committed by Alexandre Julliard Sep 06, 2017
Show whitespace changes
Inline Side-by-side

Showing with 27 additions and 22 deletions

threadpool.c dlls/ntdll/threadpool.c +27 -22

No files found.
--- 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 );
+        status = NtCreateEvent( &LocalEvent, 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 );
+        interlocked_xchg_ptr( &wait_work_item->CompletionEvent, LocalEvent );
    }
-            else
+    else if (CompletionEvent != NULL)
    {
        interlocked_xchg_ptr( &wait_work_item->CompletionEvent, CompletionEvent );
-                if (wait_work_item->CallbackInProgress)
-                    status = STATUS_PENDING;
-            }
-        }
-        else
-            status = STATUS_PENDING;
    }

+    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;
 }