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().

rtworkq: Avoid possible scenarios that an async callback could be called twice.
7865026f · Zhiyi Zhang · Alexandre Julliard · f3bab6c5 · 7865026f
Commit 7865026f authored Feb 23, 2024 by Zhiyi Zhang Committed by Alexandre Julliard Mar 04, 2024
Hide whitespace changes
Inline Side-by-side

Showing with 11 additions and 7 deletions

queue.c dlls/rtworkq/queue.c +11 -7

No files found.
--- 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);
 }