Zhiyi Zhang : rtworkq: Avoid possible scenarios that an async callback could be called twice.

[Alexandre Julliard]

Module: wine
Branch: master
Commit: 7865026f53b295eee910b51f93e56e8e1d0bb9a7
URL:    https://gitlab.winehq.org/wine/wine/-/commit/7865026f53b295eee910b51f93e56e8...

Author: Zhiyi Zhang <zzhang(a)codeweavers.com>
Date:   Fri Feb 23 17:12:04 2024 +0800

rtworkq: Avoid possible scenarios that an async callback could be called twice.

Consider a thread A running waiting_item_cancelable_callback() and a thread B running queue_cancel_item(),
which is the scenario from canceling a work item right after it gets submitted by RtwqPutWaitingWorkItem().
When the invoke_async_callback() call in queue_cancel_item() for item key with WAIT_ITEM_KEY_MASK in
thread B runs before the queue_release_pending_item() in waiting_item_cancelable_callback() in thread
A, the async callback is called the first time in queue_cancel_item() with RTWQ_E_OPERATION_CANCELLED,
then a second time in waiting_item_cancelable_callback(). We should check in queue_release_pending_item()
whether an item is already removed by queue_cancel_item() before calling async callbacks.

A different scenario could happen for scheduled_item_cancelable_callback() with the function ends
up calling its async callback even after it has been canceled by queue_cancel_item().

---

 dlls/rtworkq/queue.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/dlls/rtworkq/queue.c b/dlls/rtworkq/queue.c
index 40471ff5234..eebb096ad31 100644
--- a/dlls/rtworkq/queue.c
+++ b/dlls/rtworkq/queue.c
@@ -730,16 +730,22 @@ static HRESULT invoke_async_callback(IRtwqAsyncResult *result)
     return hr;
 }
 
-static void queue_release_pending_item(struct work_item *item)
+/* Return TRUE when the item is actually released by this function. The item could have been already
+ * removed from pending items when it got canceled. */
+static BOOL queue_release_pending_item(struct work_item *item)
 {
+    BOOL ret = FALSE;
+
     EnterCriticalSection(&item->queue->cs);
     if (item->key)
     {
         list_remove(&item->entry);
+        ret = TRUE;
         item->key = 0;
         IUnknown_Release(&item->IUnknown_iface);
     }
     LeaveCriticalSection(&item->queue->cs);
+    return ret;
 }
 
 static void CALLBACK waiting_item_callback(TP_CALLBACK_INSTANCE *instance, void *context, TP_WAIT *wait,
@@ -761,9 +767,8 @@ static void CALLBACK waiting_item_cancelable_callback(TP_CALLBACK_INSTANCE *inst
 
     TRACE("result object %p.\n", item->result);
 
-    queue_release_pending_item(item);
-
-    invoke_async_callback(item->result);
+    if (queue_release_pending_item(item))
+        invoke_async_callback(item->result);
 
     IUnknown_Release(&item->IUnknown_iface);
 }
@@ -785,9 +790,8 @@ static void CALLBACK scheduled_item_cancelable_callback(TP_CALLBACK_INSTANCE *in
 
     TRACE("result object %p.\n", item->result);
 
-    queue_release_pending_item(item);
-
-    invoke_async_callback(item->result);
+    if (queue_release_pending_item(item))
+        invoke_async_callback(item->result);
 
     IUnknown_Release(&item->IUnknown_iface);
 }