Stefan Dösinger : ntdll: Avoid more race conditions in RtlDeregisterWaitEx.

[Alexandre Julliard]

Module: wine
Branch: master
Commit: 759e6f1d8d5c531291440299bad0c1ab5ae53d05
URL:    https://source.winehq.org/git/wine.git/?a=commit;h=759e6f1d8d5c531291440299b...

Author: Stefan Dösinger <stefan(a)codeweavers.com>
Date:   Sun Oct 28 22:14:14 2018 +0100

ntdll: Avoid more race conditions in RtlDeregisterWaitEx.

af35aada9b078f8dc71dcc85e505da8eee4571da left some issues unfixed.
Instead of running the callback and returning a retval that indicates
the callback is not running, we sometimes wait for it when we are not
supposed to.

Signed-off-by: Stefan Dösinger <stefan(a)codeweavers.com>
Signed-off-by: Alexandre Julliard <julliard(a)winehq.org>

---

 dlls/ntdll/threadpool.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/dlls/ntdll/threadpool.c b/dlls/ntdll/threadpool.c
index 1b0546a..bf7449c 100644
--- a/dlls/ntdll/threadpool.c
+++ b/dlls/ntdll/threadpool.c
@@ -81,7 +81,7 @@ struct wait_work_item
     ULONG Flags;
     HANDLE CompletionEvent;
     LONG DeleteCount;
-    BOOLEAN CallbackInProgress;
+    int CallbackInProgress;
 };
 
 struct timer_queue;
@@ -530,9 +530,14 @@ static DWORD CALLBACK wait_thread_proc(LPVOID Arg)
                     wait_work_item->Context );
                 TimerOrWaitFired = TRUE;
             }
-            wait_work_item->CallbackInProgress = TRUE;
+            interlocked_xchg( &wait_work_item->CallbackInProgress, TRUE );
+            if (wait_work_item->CompletionEvent)
+            {
+                TRACE( "Work has been canceled.\n" );
+                break;
+            }
             wait_work_item->Callback( wait_work_item->Context, TimerOrWaitFired );
-            wait_work_item->CallbackInProgress = FALSE;
+            interlocked_xchg( &wait_work_item->CallbackInProgress, FALSE );
 
             if (wait_work_item->Flags & WT_EXECUTEONLYONCE)
                 break;
@@ -546,7 +551,8 @@ static DWORD CALLBACK wait_thread_proc(LPVOID Arg)
     {
         completion_event = wait_work_item->CompletionEvent;
         delete_wait_work_item( wait_work_item );
-        if (completion_event) NtSetEvent( completion_event, NULL );
+        if (completion_event && completion_event != INVALID_HANDLE_VALUE)
+            NtSetEvent( completion_event, NULL );
     }
 
     return 0;
@@ -637,14 +643,16 @@ NTSTATUS WINAPI RtlDeregisterWaitEx(HANDLE WaitHandle, HANDLE CompletionEvent)
     struct wait_work_item *wait_work_item = WaitHandle;
     NTSTATUS status;
     HANDLE LocalEvent = NULL;
-    BOOLEAN CallbackInProgress;
+    int CallbackInProgress;
 
     TRACE( "(%p %p)\n", WaitHandle, CompletionEvent );
 
     if (WaitHandle == NULL)
         return STATUS_INVALID_HANDLE;
 
+    interlocked_xchg_ptr( &wait_work_item->CompletionEvent, INVALID_HANDLE_VALUE );
     CallbackInProgress = wait_work_item->CallbackInProgress;
+    TRACE( "callback in progress %u\n", CallbackInProgress );
     if (CompletionEvent == INVALID_HANDLE_VALUE || !CallbackInProgress)
     {
         status = NtCreateEvent( &LocalEvent, EVENT_ALL_ACCESS, NULL, NotificationEvent, FALSE );
@@ -666,6 +674,7 @@ NTSTATUS WINAPI RtlDeregisterWaitEx(HANDLE WaitHandle, HANDLE CompletionEvent)
     }
     else if (LocalEvent)
     {
+        TRACE( "Waiting for completion event\n" );
         NtWaitForSingleObject( LocalEvent, FALSE, NULL );
         status = STATUS_SUCCESS;
     }