From: Mingze Feng <1156070614@qq.com> A finalization callback may still be executing when shutdown_queue() runs: CloseThreadpoolCleanupGroupMembers() does not guarantee that finalization callbacks have completed. Such a callback then calls EnterCriticalSection() on a critical section that shutdown_queue() has just deleted, crashing with STATUS_INVALID_HANDLE (0xc0000008). This was observed with a Unity game using the Media Foundation video pipeline, where minidumps from repeated crashes all pointed at the same offset in serial_queue_finalization_callback(). Track the number of finalization callbacks in flight per queue. Each work item submitted to a thread pool increments the counter of the queue owning the finalization callback, and a wrapper around the callback decrements it when the callback has returned. shutdown_queue() then waits for the counter to reach zero before deleting the critical section. Signed-off-by: Mingze Feng <1156070614@qq.com> --- dlls/rtworkq/queue.c | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/dlls/rtworkq/queue.c b/dlls/rtworkq/queue.c index 144feb98c55..aadff4312a7 100644 --- a/dlls/rtworkq/queue.c +++ b/dlls/rtworkq/queue.c @@ -198,6 +198,10 @@ struct queue /* Data used for serial queues only. */ PTP_SIMPLE_CALLBACK finalization_callback; DWORD target_queue; + /* Number of finalization callbacks that are still in flight. Shutdown has to wait + * for them to finish before the critical section can be deleted. */ + LONG inflight; + HANDLE inflight_event; }; static void shutdown_queue(struct queue *queue); @@ -393,6 +397,20 @@ static void CALLBACK standard_queue_worker(TP_CALLBACK_INSTANCE *instance, void IUnknown_Release(&item->IUnknown_iface); } +static void CALLBACK pool_queue_finalization_callback(PTP_CALLBACK_INSTANCE instance, void *context) +{ + struct work_item *item = context; + /* The finalization callback below may release the last reference to the item, + * so snapshot the queue before calling it. */ + struct queue *queue = item->queue; + + if (item->finalization_callback) + item->finalization_callback(instance, context); + + if (!InterlockedDecrement(&queue->inflight) && queue->inflight_event) + SetEvent(queue->inflight_event); +} + static void pool_queue_submit(struct queue *queue, struct work_item *item) { TP_CALLBACK_PRIORITY callback_priority; @@ -406,11 +424,14 @@ static void pool_queue_submit(struct queue *queue, struct work_item *item) callback_priority = TP_CALLBACK_PRIORITY_HIGH; env = queue->envs[callback_priority]; - env.FinalizationCallback = item->finalization_callback; + env.FinalizationCallback = pool_queue_finalization_callback; /* Worker pool callback will release one reference. Grab one more to keep object alive when we need finalization callback. */ if (item->finalization_callback) IUnknown_AddRef(&item->IUnknown_iface); + /* Count the finalization callback as in flight, so that queue shutdown can wait + * for it before deleting the critical section the callback is going to enter. */ + InterlockedIncrement(&item->queue->inflight); item->u.work_object = CreateThreadpoolWork(standard_queue_worker, item, (TP_CALLBACK_ENVIRON *)&env); item->type = WORK_ITEM_WORK; SubmitThreadpoolWork(item->u.work_object); @@ -633,6 +654,7 @@ static void init_work_queue(const struct queue_desc *desc, struct queue *queue) { list_init(&queue->pending_items); InitializeCriticalSection(&queue->cs); + queue->inflight_event = CreateEventW(NULL, TRUE, FALSE, NULL); } } @@ -699,6 +721,12 @@ static void shutdown_queue(struct queue *queue) } LeaveCriticalSection(&queue->cs); + /* A finalization callback could still be executing at this point, waiting for it to + * complete before deleting the critical section it is going to enter. */ + if (queue->inflight) + WaitForSingleObject(queue->inflight_event, INFINITE); + CloseHandle(queue->inflight_event); + DeleteCriticalSection(&queue->cs); memset(queue, 0, sizeof(*queue)); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11605