From: Conor McCarthy cmccarthy@codeweavers.com
Caching while the platform is started is consistent with platform lock count behaviour observed in Windows. --- dlls/mfplat/tests/mfplat.c | 7 --- dlls/rtworkq/queue.c | 88 +++++++++++++++++++++++++++++++++--- dlls/rtworkq/tests/rtworkq.c | 8 ---- 3 files changed, 82 insertions(+), 21 deletions(-)
diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index a31cdb4a2dc..58cf8fa7945 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -4036,13 +4036,11 @@ void test_startup_counts(void) IMFAsyncResult_Release(result2); /* Platform lock count for AsyncResult objects does not decrease * unless the platform is in shutdown state. */ - todo_wine check_platform_lock_count(3);
hr = MFCreateAsyncResult(NULL, &callback->IMFAsyncCallback_iface, NULL, &result); ok(hr == S_OK, "Failed to create result, hr %#lx.\n", hr); /* Platform lock count tracks the maximum AsyncResult count plus one for startup. */ - todo_wine check_platform_lock_count(3); IMFAsyncResult_Release(result);
@@ -4054,14 +4052,12 @@ void test_startup_counts(void) /* Release of an internal lock occurs in a worker thread. */ flaky_wine ok(!refcount, "Unexpected refcount %ld.\n", refcount); - todo_wine check_platform_lock_count(3);
hr = MFLockPlatform(); ok(hr == S_OK, "Failed to lock, %#lx.\n", hr); hr = MFLockPlatform(); ok(hr == S_OK, "Failed to lock, %#lx.\n", hr); - todo_wine check_platform_lock_count(5);
hr = MFShutdown(); @@ -4123,7 +4119,6 @@ void test_startup_counts(void) check_platform_lock_count(2); /* Release an AsyncResult object after shutdown and startup */ IMFAsyncResult_Release(result); - todo_wine check_platform_lock_count(2);
hr = MFScheduleWorkItem(&callback->IMFAsyncCallback_iface, NULL, -5000, &key); @@ -4140,12 +4135,10 @@ void test_startup_counts(void) ok(hr == S_OK, "Failed to cancel item, hr %#lx.\n", hr); hr = MFCancelWorkItem(key2); ok(hr == S_OK, "Failed to cancel item, hr %#lx.\n", hr); - todo_wine check_platform_lock_count(3);
hr = MFScheduleWorkItem(&callback->IMFAsyncCallback_iface, NULL, -5000, &key); ok(hr == S_OK, "Failed to schedule item, hr %#lx.\n", hr); - todo_wine check_platform_lock_count(3);
hr = MFShutdown(); diff --git a/dlls/rtworkq/queue.c b/dlls/rtworkq/queue.c index 2250f66bbb9..174cd509e4d 100644 --- a/dlls/rtworkq/queue.c +++ b/dlls/rtworkq/queue.c @@ -66,6 +66,16 @@ static CRITICAL_SECTION_DEBUG queues_critsect_debug = }; static CRITICAL_SECTION queues_section = { &queues_critsect_debug, -1, 0, 0, 0, 0 };
+static CRITICAL_SECTION async_result_cache_section; +static CRITICAL_SECTION_DEBUG async_result_cache_critsect_debug = +{ + 0, 0, &async_result_cache_section, + { &async_result_cache_critsect_debug.ProcessLocksList, &async_result_cache_critsect_debug.ProcessLocksList }, + 0, 0, { (DWORD_PTR)(__FILE__ ": async_result_cache_section") } +}; +static CRITICAL_SECTION async_result_cache_section = { &async_result_cache_critsect_debug, -1, 0, 0, 0, 0 }; +struct list async_result_cache = LIST_INIT(async_result_cache); + static LONG startup_count; static LONG platform_lock; static CO_MTA_USAGE_COOKIE mta_cookie; @@ -1002,6 +1012,7 @@ struct async_result LONG refcount; IUnknown *object; IUnknown *state; + struct list entry; };
static struct async_result *impl_from_IRtwqAsyncResult(IRtwqAsyncResult *iface) @@ -1009,6 +1020,68 @@ static struct async_result *impl_from_IRtwqAsyncResult(IRtwqAsyncResult *iface) return CONTAINING_RECORD(iface, struct async_result, result.AsyncResult); }
+static void free_async_result(struct async_result *result) +{ + free(result); + RtwqUnlockPlatform(); +} + +/* Async result cache. The main reason for caching is to match the behaviour in Windows. + * The platform is locked for each async result created and not unlocked until shutdown. + * This is consistent with caching of async results, which is probably done to avoid + * frequent heap allocations. It's unclear how much is gained from that here, but there + * is probably some benefit. Robustness against use-after-free is not likely to be a + * major reason because Windows seems to pop the most recently released object for the + * next allocation, as is done here, instead of popping from the tail. */ + +static BOOL async_result_cache_push(struct async_result *result) +{ + BOOL pushed; + + EnterCriticalSection(&async_result_cache_section); + if ((pushed = startup_count > 0)) + list_add_head(&async_result_cache, &result->entry); + LeaveCriticalSection(&async_result_cache_section); + + return pushed; +} + +static struct async_result *async_result_cache_pop(void) +{ + struct async_result *result; + struct list *head; + + EnterCriticalSection(&async_result_cache_section); + + if ((head = list_head(&async_result_cache))) + list_remove(head); + + LeaveCriticalSection(&async_result_cache_section); + + if (!head) + return NULL; + + result = LIST_ENTRY(head, struct async_result, entry); + memset(&result->result, 0, sizeof(result->result)); + + return result; +} + +static void async_result_cache_clear(void) +{ + struct async_result *cur, *cur2; + + EnterCriticalSection(&async_result_cache_section); + + LIST_FOR_EACH_ENTRY_SAFE(cur, cur2, &async_result_cache, struct async_result, entry) + { + list_remove(&cur->entry); + free_async_result(cur); + } + + LeaveCriticalSection(&async_result_cache_section); +} + static HRESULT WINAPI async_result_QueryInterface(IRtwqAsyncResult *iface, REFIID riid, void **obj) { TRACE("%p, %s, %p.\n", iface, debugstr_guid(riid), obj); @@ -1053,9 +1126,9 @@ static ULONG WINAPI async_result_Release(IRtwqAsyncResult *iface) IUnknown_Release(result->state); if (result->result.hEvent) CloseHandle(result->result.hEvent); - free(result);
- RtwqUnlockPlatform(); + if (!async_result_cache_push(result)) + free_async_result(result); }
return refcount; @@ -1139,10 +1212,12 @@ static HRESULT create_async_result(IUnknown *object, IRtwqAsyncCallback *callbac if (!out) return E_INVALIDARG;
- if (!(result = calloc(1, sizeof(*result)))) - return E_OUTOFMEMORY; - - RtwqLockPlatform(); + if (!(result = async_result_cache_pop())) + { + if (!(result = calloc(1, sizeof(*result)))) + return E_OUTOFMEMORY; + RtwqLockPlatform(); + }
result->result.AsyncResult.lpVtbl = &async_result_vtbl; result->refcount = 1; @@ -1248,6 +1323,7 @@ HRESULT WINAPI RtwqShutdown(void) if (InterlockedExchangeAdd(&startup_count, -1) == 1) { shutdown_system_queues(); + async_result_cache_clear(); RtwqUnlockPlatform(); }
diff --git a/dlls/rtworkq/tests/rtworkq.c b/dlls/rtworkq/tests/rtworkq.c index 8d89a5047a4..a91a4d978be 100644 --- a/dlls/rtworkq/tests/rtworkq.c +++ b/dlls/rtworkq/tests/rtworkq.c @@ -337,13 +337,11 @@ static void test_work_queue(void) IRtwqAsyncResult_Release(result2); /* Platform lock count for AsyncResult objects does not decrease * unless the platform is in shutdown state. */ - todo_wine check_platform_lock_count(3);
hr = RtwqCreateAsyncResult(NULL, &test_callback->IRtwqAsyncCallback_iface, NULL, &result); ok(hr == S_OK, "Failed to create result, hr %#lx.\n", hr); /* Platform lock count tracks the maximum AsyncResult count plus one for startup. */ - todo_wine check_platform_lock_count(3);
hr = RtwqPutWorkItem(RTWQ_CALLBACK_QUEUE_STANDARD, 0, result); @@ -353,16 +351,13 @@ static void test_work_queue(void) /* TODO: Wine often has a release call pending in another thread at this point. */ refcount = IRtwqAsyncResult_Release(result); flaky_wine - todo_wine ok(!refcount, "Unexpected refcount %ld.\n", refcount); - todo_wine check_platform_lock_count(3);
hr = RtwqLockPlatform(); ok(hr == S_OK, "Failed to lock, %#lx.\n", hr); hr = RtwqLockPlatform(); ok(hr == S_OK, "Failed to lock, %#lx.\n", hr); - todo_wine check_platform_lock_count(5);
hr = RtwqShutdown(); @@ -424,7 +419,6 @@ static void test_work_queue(void) check_platform_lock_count(2); /* Release an AsyncResult object after shutdown and startup. */ IRtwqAsyncResult_Release(result); - todo_wine check_platform_lock_count(2);
hr = RtwqCreateAsyncResult(NULL, &test_callback->IRtwqAsyncCallback_iface, NULL, &result); @@ -445,14 +439,12 @@ static void test_work_queue(void) ok(hr == S_OK, "Failed to cancel item, hr %#lx.\n", hr); IRtwqAsyncResult_Release(result); IRtwqAsyncResult_Release(result2); - todo_wine check_platform_lock_count(3);
hr = RtwqCreateAsyncResult(NULL, &test_callback->IRtwqAsyncCallback_iface, NULL, &result); ok(hr == S_OK, "Failed to create result, hr %#lx.\n", hr); hr = RtwqScheduleWorkItem(result, -5000, &key); ok(hr == S_OK, "Failed to schedule item, hr %#lx.\n", hr); - todo_wine check_platform_lock_count(3);
hr = RtwqShutdown();