[PATCH v7 0/12] MR7174: rtworkq: Fix handling of platform startup and lock counts.
Startup and lock counts in native Windows are separate, and the lock count does not decrease when an async result is freed if the platform has been started. -- v7: rtworkq: Do not cancel pending callbacks when closing a thread pool. rtworkq/tests: Test queue shutdown with pending items. rtworkq/tests: Test scheduled items. mfplat/tests: Check for async result release after canceling work and shutting down. mfplat/tests: Release callback2 in test_event_queue(). mfplat: Free the inner cookie passed to resolver_create_cancel_object(). https://gitlab.winehq.org/wine/wine/-/merge_requests/7174
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- dlls/mfplat/tests/mfplat.c | 53 ++++++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 16 deletions(-) diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index 2b17b069d1d..df45e369928 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -219,6 +219,39 @@ static void check_attributes_(const char *file, int line, IMFAttributes *attribu } } +#define check_platform_lock_count(a) check_platform_lock_count_(__LINE__, a) +static void check_platform_lock_count_(unsigned int line, unsigned int expected) +{ + int i, count = 0; + BOOL unexpected; + DWORD queue; + HRESULT hr; + + for (;;) + { + if (FAILED(hr = MFAllocateWorkQueue(&queue))) + { + unexpected = hr != MF_E_SHUTDOWN; + break; + } + MFUnlockWorkQueue(queue); + + hr = MFUnlockPlatform(); + if ((unexpected = FAILED(hr))) + break; + + ++count; + } + + for (i = 0; i < count; ++i) + MFLockPlatform(); + + if (unexpected) + count = -1; + + ok_(__FILE__, line)(count == expected, "Unexpected lock count %d.\n", count); +} + struct d3d9_surface_readback { IDirect3DSurface9 *surface, *readback_surface; @@ -3834,10 +3867,7 @@ static void test_startup(void) hr = MFStartup(MF_VERSION, MFSTARTUP_FULL); ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); - hr = MFAllocateWorkQueue(&queue); - ok(hr == S_OK, "Failed to allocate a queue, hr %#lx.\n", hr); - hr = MFUnlockWorkQueue(queue); - ok(hr == S_OK, "Failed to unlock the queue, hr %#lx.\n", hr); + check_platform_lock_count(1); hr = MFShutdown(); ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); @@ -3852,10 +3882,7 @@ static void test_startup(void) hr = MFStartup(MF_VERSION, MFSTARTUP_FULL); ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); - hr = MFAllocateWorkQueue(&queue); - ok(hr == S_OK, "Failed to allocate a queue, hr %#lx.\n", hr); - hr = MFUnlockWorkQueue(queue); - ok(hr == S_OK, "Failed to unlock the queue, hr %#lx.\n", hr); + check_platform_lock_count(1); hr = MFShutdown(); ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); @@ -3864,10 +3891,7 @@ static void test_startup(void) hr = MFStartup(MF_VERSION, MFSTARTUP_FULL); ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); - hr = MFAllocateWorkQueue(&queue); - ok(hr == S_OK, "Failed to allocate a queue, hr %#lx.\n", hr); - hr = MFUnlockWorkQueue(queue); - ok(hr == S_OK, "Failed to unlock the queue, hr %#lx.\n", hr); + check_platform_lock_count(1); /* Unlocking implies shutdown. */ hr = MFUnlockPlatform(); @@ -3879,10 +3903,7 @@ static void test_startup(void) hr = MFLockPlatform(); ok(hr == S_OK, "Failed to lock, %#lx.\n", hr); - hr = MFAllocateWorkQueue(&queue); - ok(hr == S_OK, "Failed to allocate a queue, hr %#lx.\n", hr); - hr = MFUnlockWorkQueue(queue); - ok(hr == S_OK, "Failed to unlock the queue, hr %#lx.\n", hr); + check_platform_lock_count(1); hr = MFShutdown(); ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7174
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- dlls/mfplat/tests/mfplat.c | 76 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index df45e369928..f1267308ee1 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -42,6 +42,7 @@ #include "evr.h" #include "mfmediaengine.h" #include "codecapi.h" +#include "rtworkq.h" #include "wine/test.h" @@ -542,6 +543,11 @@ static HRESULT (WINAPI *pMFLockDXGIDeviceManager)(UINT *token, IMFDXGIDeviceMana static HRESULT (WINAPI *pMFUnlockDXGIDeviceManager)(void); static HRESULT (WINAPI *pMFInitVideoFormat_RGB)(MFVIDEOFORMAT *format, DWORD width, DWORD height, DWORD d3dformat); +static HRESULT (WINAPI *pRtwqStartup)(void); +static HRESULT (WINAPI *pRtwqShutdown)(void); +static HRESULT (WINAPI *pRtwqLockPlatform)(void); +static HRESULT (WINAPI *pRtwqUnlockPlatform)(void); + static HWND create_window(void) { RECT r = {0, 0, 640, 480}; @@ -1752,6 +1758,14 @@ static void init_functions(void) mod = GetModuleHandleA("ole32.dll"); X(CoGetApartmentType); + + if ((mod = LoadLibraryA("rtworkq.dll"))) + { + X(RtwqStartup); + X(RtwqShutdown); + X(RtwqUnlockPlatform); + X(RtwqLockPlatform); + } #undef X is_win8_plus = pMFPutWaitingWorkItem != NULL; @@ -3858,6 +3872,8 @@ static void test_MFCreateAsyncResult(void) static void test_startup(void) { + struct test_callback *callback; + IMFAsyncResult *result; DWORD queue; HRESULT hr; @@ -3907,6 +3923,66 @@ static void test_startup(void) hr = MFShutdown(); ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); + + if (!pRtwqStartup) + { + win_skip("RtwqStartup() not found.\n"); + return; + } + + /* Rtwq equivalence */ + + hr = MFStartup(MF_VERSION, MFSTARTUP_FULL); + ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); + + hr = pRtwqStartup(); + ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); + hr = pRtwqShutdown(); + ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); + check_platform_lock_count(1); + + /* Matching MFStartup() with RtwqShutdown() causes shutdown. */ + hr = pRtwqShutdown(); + ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); + + hr = MFAllocateWorkQueue(&queue); + ok(hr == MF_E_SHUTDOWN, "Failed to allocate a queue, hr %#lx.\n", hr); + + hr = MFShutdown(); + ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); + + /* RtwqStartup() enables MF functions */ + hr = pRtwqStartup(); + ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); + + check_platform_lock_count(1); + + callback = create_test_callback(NULL); + + /* MF platform lock is the Rtwq lock */ + hr = pRtwqUnlockPlatform(); + ok(hr == S_OK, "Failed to unlock platform, hr %#lx.\n", hr); + + hr = MFAllocateWorkQueue(&queue); + ok(hr == MF_E_SHUTDOWN, "Failed to allocate a queue, hr %#lx.\n", hr); + + hr = MFCreateAsyncResult(NULL, &callback->IMFAsyncCallback_iface, NULL, &result); + ok(hr == S_OK, "Failed to create result, hr %#lx.\n", hr); + check_platform_lock_count(1); + + hr = pRtwqLockPlatform(); + ok(hr == S_OK, "Failed to lock platform, hr %#lx.\n", hr); + check_platform_lock_count(2); + + IMFAsyncResult_Release(result); + + hr = pRtwqShutdown(); + ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); + + hr = MFAllocateWorkQueue(&queue); + ok(hr == MF_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); + + IMFAsyncCallback_Release(&callback->IMFAsyncCallback_iface); } static void test_allocate_queue(void) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7174
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- dlls/mfplat/tests/mfplat.c | 194 +++++++++++++++++++++++++++++++++++++ 1 file changed, 194 insertions(+) diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index f1267308ee1..18a53ef0cee 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -3985,6 +3985,199 @@ static void test_startup(void) IMFAsyncCallback_Release(&callback->IMFAsyncCallback_iface); } +void test_startup_counts(void) +{ + IMFAsyncResult *result, *result2, *callback_result; + struct test_callback *callback; + MFWORKITEM_KEY key, key2; + DWORD res, queue; + LONG refcount; + HRESULT hr; + + hr = MFLockPlatform(); + ok(hr == S_OK, "Failed to lock, %#lx.\n", hr); + hr = MFAllocateWorkQueue(&queue); + todo_wine + ok(hr == MF_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); + hr = MFUnlockPlatform(); + ok(hr == S_OK, "Failed to unlock, %#lx.\n", hr); + + callback = create_test_callback(&test_async_callback_result_vtbl); + + /* Create async results without startup. */ + hr = MFCreateAsyncResult(NULL, &callback->IMFAsyncCallback_iface, NULL, &result); + ok(hr == S_OK, "Failed to create result, hr %#lx.\n", hr); + hr = MFCreateAsyncResult(NULL, &callback->IMFAsyncCallback_iface, NULL, &result2); + ok(hr == S_OK, "Failed to create result, hr %#lx.\n", hr); + IMFAsyncResult_Release(result); + IMFAsyncResult_Release(result2); + + hr = MFStartup(MF_VERSION, MFSTARTUP_FULL); + ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); + /* Before startup the platform lock count does not track the maximum AsyncResult count. */ + check_platform_lock_count(1); + + hr = MFStartup(MF_VERSION, MFSTARTUP_FULL); + ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); + /* Startup only locks once. */ + todo_wine + check_platform_lock_count(1); + hr = MFShutdown(); + ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); + + hr = MFCreateAsyncResult(NULL, &callback->IMFAsyncCallback_iface, NULL, &result); + ok(hr == S_OK, "Failed to create result, hr %#lx.\n", hr); + /* Platform locked by the AsyncResult object. */ + check_platform_lock_count(2); + + hr = MFCreateAsyncResult(NULL, &callback->IMFAsyncCallback_iface, NULL, &result2); + ok(hr == S_OK, "Failed to create result, hr %#lx.\n", hr); + check_platform_lock_count(3); + + IMFAsyncResult_Release(result); + 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); + + hr = MFPutWorkItem(MFASYNC_CALLBACK_QUEUE_STANDARD, &callback->IMFAsyncCallback_iface, NULL); + ok(hr == S_OK, "got %#lx\n", hr); + res = wait_async_callback_result(&callback->IMFAsyncCallback_iface, 100, &callback_result); + ok(res == 0, "got %#lx\n", res); + refcount = IMFAsyncResult_Release(callback_result); + /* 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(); + ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); + + /* Platform is in shutdown state if either the lock count or the startup count is <= 0. */ + hr = MFAllocateWorkQueue(&queue); + todo_wine + ok(hr == MF_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); + + /* Platform can be unlocked after shutdown. */ + hr = MFUnlockPlatform(); + ok(hr == S_OK, "Failed to unlock, %#lx.\n", hr); + + hr = MFStartup(MF_VERSION, MFSTARTUP_FULL); + ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); + + /* Platform locks for AsyncResult objects were released on shutdown, but the explicit lock was not. */ + check_platform_lock_count(2); + hr = MFUnlockPlatform(); + ok(hr == S_OK, "Failed to unlock, %#lx.\n", hr); + check_platform_lock_count(1); + + hr = MFUnlockPlatform(); + ok(hr == S_OK, "Failed to unlock, %#lx.\n", hr); + /* Zero lock count. */ + hr = MFAllocateWorkQueue(&queue); + ok(hr == MF_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); + hr = MFUnlockPlatform(); + ok(hr == S_OK, "Failed to unlock, %#lx.\n", hr); + /* Negative lock count. */ + hr = MFAllocateWorkQueue(&queue); + ok(hr == MF_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); + hr = MFLockPlatform(); + ok(hr == S_OK, "Failed to lock, %#lx.\n", hr); + hr = MFLockPlatform(); + ok(hr == S_OK, "Failed to lock, %#lx.\n", hr); + check_platform_lock_count(1); + + hr = MFCreateAsyncResult(NULL, &callback->IMFAsyncCallback_iface, NULL, &result); + ok(hr == S_OK, "Failed to create result, hr %#lx.\n", hr); + check_platform_lock_count(2); + + hr = MFShutdown(); + ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); + + /* Release an AsyncResult object after shutdown. Lock count tracks the AsyncResult count. + * It's not possible to show if unlock occurs immedately or on the next startup. */ + IMFAsyncResult_Release(result); + hr = MFStartup(MF_VERSION, MFSTARTUP_FULL); + ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); + check_platform_lock_count(1); + + hr = MFCreateAsyncResult(NULL, &callback->IMFAsyncCallback_iface, NULL, &result); + ok(hr == S_OK, "Failed to create result, hr %#lx.\n", hr); + hr = MFShutdown(); + ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); + hr = MFStartup(MF_VERSION, MFSTARTUP_FULL); + ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); + 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); + ok(hr == S_OK, "Failed to schedule item, hr %#lx.\n", hr); + /* The AsyncResult created for the item locks the platform */ + check_platform_lock_count(2); + + hr = MFScheduleWorkItem(&callback->IMFAsyncCallback_iface, NULL, -5000, &key2); + ok(hr == S_OK, "Failed to schedule item, hr %#lx.\n", hr); + check_platform_lock_count(3); + + /* Platform locks for scheduled items are not released */ + hr = MFCancelWorkItem(key); + 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(); + ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); + + hr = MFAllocateWorkQueue(&queue); + todo_wine + ok(hr == MF_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); + + hr = MFCancelWorkItem(key); + todo_wine + ok(hr == MF_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); + + res = wait_async_callback_result(&callback->IMFAsyncCallback_iface, 0, &result); + ok(res == WAIT_TIMEOUT, "got res %#lx\n", res); + + hr = MFStartup(MF_VERSION, MFSTARTUP_FULL); + ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); + + /* Shutdown while a scheduled item is pending leaks the internal AsyncResult. */ + todo_wine + check_platform_lock_count(2); + + hr = MFShutdown(); + ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); + + IMFAsyncCallback_Release(&callback->IMFAsyncCallback_iface); +} + static void test_allocate_queue(void) { DWORD queue, queue2; @@ -13352,6 +13545,7 @@ START_TEST(mfplat) CoInitialize(NULL); test_startup(); + test_startup_counts(); test_register(); test_media_type(); test_MFCreateMediaEvent(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7174
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- dlls/rtworkq/tests/rtworkq.c | 337 +++++++++++++++++++++++++++++++++++ 1 file changed, 337 insertions(+) diff --git a/dlls/rtworkq/tests/rtworkq.c b/dlls/rtworkq/tests/rtworkq.c index 2ff74d57b84..e7d59bae656 100644 --- a/dlls/rtworkq/tests/rtworkq.c +++ b/dlls/rtworkq/tests/rtworkq.c @@ -19,12 +19,156 @@ #include <stdarg.h> #include <string.h> +#define COBJMACROS + #include "windef.h" #include "winbase.h" +#include "initguid.h" #include "rtworkq.h" #include "wine/test.h" +/* Should be kept in sync with corresponding MFASYNC_CALLBACK_ constants. */ +enum rtwq_callback_queue_id +{ + RTWQ_CALLBACK_QUEUE_UNDEFINED = 0x00000000, + RTWQ_CALLBACK_QUEUE_STANDARD = 0x00000001, + RTWQ_CALLBACK_QUEUE_RT = 0x00000002, + RTWQ_CALLBACK_QUEUE_IO = 0x00000003, + RTWQ_CALLBACK_QUEUE_TIMER = 0x00000004, + RTWQ_CALLBACK_QUEUE_MULTITHREADED = 0x00000005, + RTWQ_CALLBACK_QUEUE_LONG_FUNCTION = 0x00000007, + RTWQ_CALLBACK_QUEUE_PRIVATE_MASK = 0xffff0000, + RTWQ_CALLBACK_QUEUE_ALL = 0xffffffff, +}; + +#define check_platform_lock_count(a) check_platform_lock_count_(__LINE__, a) +static void check_platform_lock_count_(unsigned int line, unsigned int expected) +{ + int i, count = 0; + BOOL unexpected; + DWORD queue; + HRESULT hr; + + for (;;) + { + if (FAILED(hr = RtwqAllocateWorkQueue(RTWQ_STANDARD_WORKQUEUE, &queue))) + { + unexpected = hr != RTWQ_E_SHUTDOWN; + break; + } + RtwqUnlockWorkQueue(queue); + + hr = RtwqUnlockPlatform(); + if ((unexpected = FAILED(hr))) + break; + + ++count; + } + + for (i = 0; i < count; ++i) + RtwqLockPlatform(); + + if (unexpected) + count = -1; + + ok_(__FILE__, line)(count == expected, "Unexpected lock count %d.\n", count); +} + +struct test_callback +{ + IRtwqAsyncCallback IRtwqAsyncCallback_iface; + LONG refcount; + HANDLE event; + IRtwqAsyncResult *result; +}; + +static struct test_callback *impl_from_IRtwqAsyncCallback(IRtwqAsyncCallback *iface) +{ + return CONTAINING_RECORD(iface, struct test_callback, IRtwqAsyncCallback_iface); +} + +static HRESULT WINAPI testcallback_QueryInterface(IRtwqAsyncCallback *iface, REFIID riid, void **obj) +{ + if (IsEqualIID(riid, &IID_IRtwqAsyncCallback) || + IsEqualIID(riid, &IID_IUnknown)) + { + *obj = iface; + IRtwqAsyncCallback_AddRef(iface); + return S_OK; + } + + *obj = NULL; + return E_NOINTERFACE; +} + +static ULONG WINAPI testcallback_AddRef(IRtwqAsyncCallback *iface) +{ + struct test_callback *callback = impl_from_IRtwqAsyncCallback(iface); + return InterlockedIncrement(&callback->refcount); +} + +static ULONG WINAPI testcallback_Release(IRtwqAsyncCallback *iface) +{ + struct test_callback *callback = impl_from_IRtwqAsyncCallback(iface); + ULONG refcount = InterlockedDecrement(&callback->refcount); + + if (!refcount) + { + CloseHandle(callback->event); + free(callback); + } + + return refcount; +} + +static HRESULT WINAPI testcallback_GetParameters(IRtwqAsyncCallback *iface, DWORD *flags, DWORD *queue) +{ + ok(flags != NULL && queue != NULL, "Unexpected arguments.\n"); + return E_NOTIMPL; +} + +static HRESULT WINAPI testcallback_Invoke(IRtwqAsyncCallback *iface, IRtwqAsyncResult *result) +{ + struct test_callback *callback = impl_from_IRtwqAsyncCallback(iface); + + callback->result = result; + SetEvent(callback->event); + + return S_OK; +} + +static const IRtwqAsyncCallbackVtbl testcallbackvtbl = +{ + testcallback_QueryInterface, + testcallback_AddRef, + testcallback_Release, + testcallback_GetParameters, + testcallback_Invoke, +}; + +static struct test_callback * create_test_callback(void) +{ + struct test_callback *callback = calloc(1, sizeof(*callback)); + + callback->IRtwqAsyncCallback_iface.lpVtbl = &testcallbackvtbl; + callback->refcount = 1; + callback->event = CreateEventA(NULL, FALSE, FALSE, NULL); + + return callback; +} + +static DWORD wait_async_callback_result(IRtwqAsyncCallback *iface, DWORD timeout, IRtwqAsyncResult **result) +{ + struct test_callback *callback = impl_from_IRtwqAsyncCallback(iface); + DWORD res = WaitForSingleObject(callback->event, timeout); + + *result = callback->result; + callback->result = NULL; + + return res; +} + static void test_platform_init(void) { APTTYPEQUALIFIER qualifier; @@ -101,7 +245,200 @@ static void test_platform_init(void) ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); } +static void test_work_queue(void) +{ + IRtwqAsyncResult *result, *result2, *callback_result; + struct test_callback *test_callback; + RTWQWORKITEM_KEY key, key2; + DWORD res, queue; + LONG refcount; + HRESULT hr; + + hr = RtwqLockPlatform(); + ok(hr == S_OK, "Failed to lock, %#lx.\n", hr); + hr = RtwqAllocateWorkQueue(RTWQ_STANDARD_WORKQUEUE, &queue); + todo_wine + ok(hr == RTWQ_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); + hr = RtwqUnlockPlatform(); + ok(hr == S_OK, "Failed to unlock, %#lx.\n", hr); + + test_callback = create_test_callback(); + + /* Create async results without startup. */ + hr = RtwqCreateAsyncResult(NULL, &test_callback->IRtwqAsyncCallback_iface, NULL, &result); + ok(hr == S_OK, "Failed to create result, hr %#lx.\n", hr); + hr = RtwqCreateAsyncResult(NULL, &test_callback->IRtwqAsyncCallback_iface, NULL, &result2); + ok(hr == S_OK, "Failed to create result, hr %#lx.\n", hr); + IRtwqAsyncResult_Release(result); + IRtwqAsyncResult_Release(result2); + + hr = RtwqStartup(); + ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); + /* Before startup the platform lock count does not track the maximum AsyncResult count. */ + check_platform_lock_count(1); + + hr = RtwqStartup(); + ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); + /* Startup only locks once. */ + todo_wine + check_platform_lock_count(1); + hr = RtwqShutdown(); + ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); + + hr = RtwqCreateAsyncResult(NULL, &test_callback->IRtwqAsyncCallback_iface, NULL, &result); + ok(hr == S_OK, "Failed to create result, hr %#lx.\n", hr); + /* Platform locked by the AsyncResult object. */ + check_platform_lock_count(2); + + hr = RtwqCreateAsyncResult(NULL, &test_callback->IRtwqAsyncCallback_iface, NULL, &result2); + ok(hr == S_OK, "Failed to create result, hr %#lx.\n", hr); + check_platform_lock_count(3); + + IRtwqAsyncResult_Release(result); + 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); + ok(hr == S_OK, "got %#lx\n", hr); + res = wait_async_callback_result(&test_callback->IRtwqAsyncCallback_iface, 100, &callback_result); + ok(res == 0, "got %#lx\n", res); + /* 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(); + ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); + + /* Platform is in shutdown state if either the lock count or the startup count is <= 0. */ + hr = RtwqAllocateWorkQueue(RTWQ_STANDARD_WORKQUEUE, &queue); + todo_wine + ok(hr == RTWQ_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); + + /* Platform can be unlocked after shutdown. */ + hr = RtwqUnlockPlatform(); + ok(hr == S_OK, "Failed to unlock, %#lx.\n", hr); + + hr = RtwqStartup(); + ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); + + /* Platform locks for AsyncResult objects were released on shutdown, but the explicit lock was not. */ + check_platform_lock_count(2); + hr = RtwqUnlockPlatform(); + ok(hr == S_OK, "Failed to unlock, %#lx.\n", hr); + check_platform_lock_count(1); + + hr = RtwqUnlockPlatform(); + ok(hr == S_OK, "Failed to unlock, %#lx.\n", hr); + /* Zero lock count. */ + hr = RtwqAllocateWorkQueue(RTWQ_STANDARD_WORKQUEUE, &queue); + ok(hr == RTWQ_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); + hr = RtwqUnlockPlatform(); + ok(hr == S_OK, "Failed to unlock, %#lx.\n", hr); + /* Negative lock count. */ + hr = RtwqAllocateWorkQueue(RTWQ_STANDARD_WORKQUEUE, &queue); + ok(hr == RTWQ_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); + hr = RtwqLockPlatform(); + ok(hr == S_OK, "Failed to lock, %#lx.\n", hr); + hr = RtwqLockPlatform(); + ok(hr == S_OK, "Failed to lock, %#lx.\n", hr); + check_platform_lock_count(1); + + hr = RtwqCreateAsyncResult(NULL, &test_callback->IRtwqAsyncCallback_iface, NULL, &result); + ok(hr == S_OK, "Failed to create result, hr %#lx.\n", hr); + check_platform_lock_count(2); + + hr = RtwqShutdown(); + ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); + + /* Release an AsyncResult object after shutdown. Platform lock count tracks the AsyncResult + * count. It's not possible to show if unlock occurs immedately or on the next startup. */ + IRtwqAsyncResult_Release(result); + hr = RtwqStartup(); + ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); + check_platform_lock_count(1); + + hr = RtwqCreateAsyncResult(NULL, &test_callback->IRtwqAsyncCallback_iface, NULL, &result); + ok(hr == S_OK, "Failed to create result, hr %#lx.\n", hr); + hr = RtwqShutdown(); + ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); + hr = RtwqStartup(); + ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); + 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); + 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); + check_platform_lock_count(2); + + hr = RtwqCreateAsyncResult(NULL, &test_callback->IRtwqAsyncCallback_iface, NULL, &result2); + ok(hr == S_OK, "Failed to create result, hr %#lx.\n", hr); + hr = RtwqScheduleWorkItem(result2, -5000, &key2); + ok(hr == S_OK, "Failed to schedule item, hr %#lx.\n", hr); + check_platform_lock_count(3); + + hr = RtwqCancelWorkItem(key); + ok(hr == S_OK, "Failed to cancel item, hr %#lx.\n", hr); + hr = RtwqCancelWorkItem(key2); + 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(); + ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); + /* Shutdown while a scheduled item is pending leaks the AsyncResult. */ + refcount = IRtwqAsyncResult_Release(result); + ok(refcount == 1, "Unexpected refcount %ld.\n", refcount); + IRtwqAsyncResult_Release(result); + + hr = RtwqAllocateWorkQueue(RTWQ_STANDARD_WORKQUEUE, &queue); + ok(hr == RTWQ_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); + + hr = RtwqCancelWorkItem(key); + todo_wine + ok(hr == RTWQ_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); + + res = wait_async_callback_result(&test_callback->IRtwqAsyncCallback_iface, 0, &callback_result); + ok(res == WAIT_TIMEOUT, "got res %#lx\n", res); + + IRtwqAsyncCallback_Release(&test_callback->IRtwqAsyncCallback_iface); +} + START_TEST(rtworkq) { test_platform_init(); + test_work_queue(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7174
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- dlls/mfplat/tests/mfplat.c | 6 ------ dlls/rtworkq/queue.c | 11 +++++++---- dlls/rtworkq/tests/rtworkq.c | 4 ---- 3 files changed, 7 insertions(+), 14 deletions(-) diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index 18a53ef0cee..102c765c251 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -3997,7 +3997,6 @@ void test_startup_counts(void) hr = MFLockPlatform(); ok(hr == S_OK, "Failed to lock, %#lx.\n", hr); hr = MFAllocateWorkQueue(&queue); - todo_wine ok(hr == MF_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); hr = MFUnlockPlatform(); ok(hr == S_OK, "Failed to unlock, %#lx.\n", hr); @@ -4020,7 +4019,6 @@ void test_startup_counts(void) hr = MFStartup(MF_VERSION, MFSTARTUP_FULL); ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); /* Startup only locks once. */ - todo_wine check_platform_lock_count(1); hr = MFShutdown(); ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); @@ -4071,7 +4069,6 @@ void test_startup_counts(void) /* Platform is in shutdown state if either the lock count or the startup count is <= 0. */ hr = MFAllocateWorkQueue(&queue); - todo_wine ok(hr == MF_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); /* Platform can be unlocked after shutdown. */ @@ -4155,11 +4152,9 @@ void test_startup_counts(void) ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); hr = MFAllocateWorkQueue(&queue); - todo_wine ok(hr == MF_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); hr = MFCancelWorkItem(key); - todo_wine ok(hr == MF_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); res = wait_async_callback_result(&callback->IMFAsyncCallback_iface, 0, &result); @@ -4169,7 +4164,6 @@ void test_startup_counts(void) ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); /* Shutdown while a scheduled item is pending leaks the internal AsyncResult. */ - todo_wine check_platform_lock_count(2); hr = MFShutdown(); diff --git a/dlls/rtworkq/queue.c b/dlls/rtworkq/queue.c index 9046a70b359..4486aafc253 100644 --- a/dlls/rtworkq/queue.c +++ b/dlls/rtworkq/queue.c @@ -66,6 +66,7 @@ static CRITICAL_SECTION_DEBUG queues_critsect_debug = }; static CRITICAL_SECTION queues_section = { &queues_critsect_debug, -1, 0, 0, 0, 0 }; +static LONG startup_count; static LONG platform_lock; static CO_MTA_USAGE_COOKIE mta_cookie; @@ -954,7 +955,7 @@ static HRESULT alloc_user_queue(const struct queue_desc *desc, DWORD *queue_id) *queue_id = RTWQ_CALLBACK_QUEUE_UNDEFINED; - if (platform_lock <= 0) + if (startup_count <= 0 || platform_lock <= 0) return RTWQ_E_SHUTDOWN; if (!(queue = calloc(1, sizeof(*queue)))) @@ -1206,8 +1207,9 @@ static void init_system_queues(void) HRESULT WINAPI RtwqStartup(void) { - if (InterlockedIncrement(&platform_lock) == 1) + if (InterlockedIncrement(&startup_count) == 1) { + RtwqLockPlatform(); init_system_queues(); } @@ -1234,12 +1236,13 @@ static void shutdown_system_queues(void) HRESULT WINAPI RtwqShutdown(void) { - if (platform_lock <= 0) + if (startup_count <= 0) return S_OK; - if (InterlockedExchangeAdd(&platform_lock, -1) == 1) + if (InterlockedExchangeAdd(&startup_count, -1) == 1) { shutdown_system_queues(); + RtwqUnlockPlatform(); } return S_OK; diff --git a/dlls/rtworkq/tests/rtworkq.c b/dlls/rtworkq/tests/rtworkq.c index e7d59bae656..32fe82b29bb 100644 --- a/dlls/rtworkq/tests/rtworkq.c +++ b/dlls/rtworkq/tests/rtworkq.c @@ -257,7 +257,6 @@ static void test_work_queue(void) hr = RtwqLockPlatform(); ok(hr == S_OK, "Failed to lock, %#lx.\n", hr); hr = RtwqAllocateWorkQueue(RTWQ_STANDARD_WORKQUEUE, &queue); - todo_wine ok(hr == RTWQ_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); hr = RtwqUnlockPlatform(); ok(hr == S_OK, "Failed to unlock, %#lx.\n", hr); @@ -280,7 +279,6 @@ static void test_work_queue(void) hr = RtwqStartup(); ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); /* Startup only locks once. */ - todo_wine check_platform_lock_count(1); hr = RtwqShutdown(); ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); @@ -331,7 +329,6 @@ static void test_work_queue(void) /* Platform is in shutdown state if either the lock count or the startup count is <= 0. */ hr = RtwqAllocateWorkQueue(RTWQ_STANDARD_WORKQUEUE, &queue); - todo_wine ok(hr == RTWQ_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); /* Platform can be unlocked after shutdown. */ @@ -428,7 +425,6 @@ static void test_work_queue(void) ok(hr == RTWQ_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); hr = RtwqCancelWorkItem(key); - todo_wine ok(hr == RTWQ_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); res = wait_async_callback_result(&test_callback->IRtwqAsyncCallback_iface, 0, &callback_result); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7174
From: Conor McCarthy <cmccarthy(a)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 102c765c251..e6a0186c231 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 4486aafc253..84ea084e2fa 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; @@ -996,6 +1006,7 @@ struct async_result LONG refcount; IUnknown *object; IUnknown *state; + struct list entry; }; static struct async_result *impl_from_IRtwqAsyncResult(IRtwqAsyncResult *iface) @@ -1003,6 +1014,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); @@ -1047,9 +1120,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; @@ -1133,10 +1206,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; @@ -1242,6 +1317,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 32fe82b29bb..05e81a3116d 100644 --- a/dlls/rtworkq/tests/rtworkq.c +++ b/dlls/rtworkq/tests/rtworkq.c @@ -296,13 +296,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); @@ -312,16 +310,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(); @@ -383,7 +378,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); @@ -404,14 +398,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(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7174
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- dlls/mfplat/main.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/dlls/mfplat/main.c b/dlls/mfplat/main.c index 4ed607686f1..e688cf9cf16 100644 --- a/dlls/mfplat/main.c +++ b/dlls/mfplat/main.c @@ -6672,8 +6672,12 @@ static HRESULT WINAPI source_resolver_BeginCreateObjectFromURL(IMFSourceResolver hr = IMFSchemeHandler_BeginCreateObject(handler, url, flags, props, cancel_cookie ? &inner_cookie : NULL, (IMFAsyncCallback *)&resolver->url_callback, (IUnknown *)result); - if (SUCCEEDED(hr) && inner_cookie) - resolver_create_cancel_object((IUnknown *)handler, OBJECT_FROM_URL, inner_cookie, cancel_cookie); + if (inner_cookie) + { + if (SUCCEEDED(hr)) + resolver_create_cancel_object((IUnknown *)handler, OBJECT_FROM_URL, inner_cookie, cancel_cookie); + IUnknown_Release(inner_cookie); + } IRtwqAsyncResult_Release(result); @@ -6717,9 +6721,13 @@ static HRESULT WINAPI source_resolver_BeginCreateObjectFromByteStream(IMFSourceR hr = IMFByteStreamHandler_BeginCreateObject(handler, stream, url, flags, props, cancel_cookie ? &inner_cookie : NULL, (IMFAsyncCallback *)&resolver->stream_callback, (IUnknown *)result); - /* Cancel object wraps underlying handler cancel cookie with context necessary to call CancelObjectCreate(). */ - if (SUCCEEDED(hr) && inner_cookie) - resolver_create_cancel_object((IUnknown *)handler, OBJECT_FROM_BYTESTREAM, inner_cookie, cancel_cookie); + if (inner_cookie) + { + /* Cancel object wraps underlying handler cancel cookie with context necessary to call CancelObjectCreate(). */ + if (SUCCEEDED(hr)) + resolver_create_cancel_object((IUnknown *)handler, OBJECT_FROM_BYTESTREAM, inner_cookie, cancel_cookie); + IUnknown_Release(inner_cookie); + } IRtwqAsyncResult_Release(result); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7174
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- dlls/mfplat/tests/mfplat.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index e6a0186c231..93d0a2386ae 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -4668,6 +4668,7 @@ static void test_event_queue(void) ok(ret == 1 || broken(ret == 2) /* Vista */, "Unexpected refcount %ld, expected 1.\n", ret); IMFAsyncCallback_Release(&callback->IMFAsyncCallback_iface); + IMFAsyncCallback_Release(&callback2->IMFAsyncCallback_iface); hr = MFShutdown(); ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7174
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- dlls/mfplat/tests/mfplat.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index 93d0a2386ae..d75849a23b3 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -4412,8 +4412,6 @@ static void test_scheduled_items(void) hr = MFCancelWorkItem(key2); ok(hr == S_OK, "Failed to cancel item, hr %#lx.\n", hr); - IMFAsyncResult_Release(result); - hr = MFScheduleWorkItem(&callback->IMFAsyncCallback_iface, NULL, -5000, &key); ok(hr == S_OK, "Failed to schedule item, hr %#lx.\n", hr); @@ -4423,6 +4421,13 @@ static void test_scheduled_items(void) hr = MFShutdown(); ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); + Sleep(20); + /* A callback invocation with RTWQ_E_OPERATION_CANCELLED may have been pending + * when MFShutdown() was called. Release depends upon its execution. */ + refcount = IMFAsyncResult_Release(result); + flaky_wine + ok(refcount == 0, "Unexpected refcount %lu.\n", refcount); + IMFAsyncCallback_Release(&callback->IMFAsyncCallback_iface); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7174
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- dlls/rtworkq/tests/rtworkq.c | 59 ++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/dlls/rtworkq/tests/rtworkq.c b/dlls/rtworkq/tests/rtworkq.c index 05e81a3116d..930e5fdc389 100644 --- a/dlls/rtworkq/tests/rtworkq.c +++ b/dlls/rtworkq/tests/rtworkq.c @@ -425,8 +425,67 @@ static void test_work_queue(void) IRtwqAsyncCallback_Release(&test_callback->IRtwqAsyncCallback_iface); } +static void test_scheduled_items(void) +{ + struct test_callback *test_callback; + RTWQWORKITEM_KEY key, key2; + IRtwqAsyncResult *result; + ULONG refcount; + HRESULT hr; + + test_callback = create_test_callback(); + + hr = RtwqStartup(); + ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); + + 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); + IRtwqAsyncResult_Release(result); + + hr = RtwqCancelWorkItem(key); + ok(hr == S_OK, "Failed to cancel item, hr %#lx.\n", hr); + + refcount = IRtwqAsyncCallback_Release(&test_callback->IRtwqAsyncCallback_iface); + ok(refcount == 0, "Unexpected refcount %lu.\n", refcount); + + hr = RtwqCancelWorkItem(key); + ok(hr == RTWQ_E_NOT_FOUND || broken(hr == S_OK) /* < win10 */, "Unexpected hr %#lx.\n", hr); + + test_callback = create_test_callback(); + + hr = RtwqCreateAsyncResult(NULL, &test_callback->IRtwqAsyncCallback_iface, NULL, &result); + ok(hr == S_OK, "Failed to create result, hr %#lx.\n", hr); + + hr = RtwqPutWaitingWorkItem(NULL, 0, result, &key); + ok(hr == S_OK, "Failed to add waiting item, hr %#lx.\n", hr); + + hr = RtwqPutWaitingWorkItem(NULL, 0, result, &key2); + ok(hr == S_OK, "Failed to add waiting item, hr %#lx.\n", hr); + + hr = RtwqCancelWorkItem(key); + ok(hr == S_OK, "Failed to cancel item, hr %#lx.\n", hr); + + hr = RtwqCancelWorkItem(key2); + ok(hr == S_OK, "Failed to cancel item, hr %#lx.\n", hr); + + hr = RtwqShutdown(); + ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); + + Sleep(20); + /* One or two callback invocations with RTWQ_E_OPERATION_CANCELLED may have been + * pending when RtwqShutdown() was called. Release depends upon their execution. */ + refcount = IRtwqAsyncResult_Release(result); + flaky_wine + ok(refcount == 0, "Unexpected refcount %lu.\n", refcount); + + IRtwqAsyncCallback_Release(&test_callback->IRtwqAsyncCallback_iface); +} + START_TEST(rtworkq) { test_platform_init(); test_work_queue(); + test_scheduled_items(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7174
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- dlls/rtworkq/tests/rtworkq.c | 85 ++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/dlls/rtworkq/tests/rtworkq.c b/dlls/rtworkq/tests/rtworkq.c index 930e5fdc389..d938be3dd76 100644 --- a/dlls/rtworkq/tests/rtworkq.c +++ b/dlls/rtworkq/tests/rtworkq.c @@ -80,6 +80,7 @@ struct test_callback IRtwqAsyncCallback IRtwqAsyncCallback_iface; LONG refcount; HANDLE event; + UINT sleep_ms; IRtwqAsyncResult *result; }; @@ -132,6 +133,9 @@ static HRESULT WINAPI testcallback_Invoke(IRtwqAsyncCallback *iface, IRtwqAsyncR { struct test_callback *callback = impl_from_IRtwqAsyncCallback(iface); + if (callback->sleep_ms) + Sleep(callback->sleep_ms); + callback->result = result; SetEvent(callback->event); @@ -483,9 +487,90 @@ static void test_scheduled_items(void) IRtwqAsyncCallback_Release(&test_callback->IRtwqAsyncCallback_iface); } +static void test_queue_shutdown(void) +{ + IRtwqAsyncResult *result, *result2, *callback_result; + struct test_callback *test_callback, *test_callback2; + DWORD res, queue; + LONG refcount; + HRESULT hr; + + hr = RtwqStartup(); + ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); + + test_callback = create_test_callback(); + test_callback2 = create_test_callback(); + test_callback->sleep_ms = 10; + test_callback2->sleep_ms = 10; + + hr = RtwqCreateAsyncResult(NULL, &test_callback->IRtwqAsyncCallback_iface, NULL, &result); + ok(hr == S_OK, "Failed to create result, hr %#lx.\n", hr); + hr = RtwqCreateAsyncResult(NULL, &test_callback2->IRtwqAsyncCallback_iface, NULL, &result2); + ok(hr == S_OK, "Failed to create result, hr %#lx.\n", hr); + hr = RtwqPutWorkItem(RTWQ_CALLBACK_QUEUE_STANDARD, 0, result); + ok(hr == S_OK, "got %#lx\n", hr); + hr = RtwqPutWorkItem(RTWQ_CALLBACK_QUEUE_STANDARD, 0, result2); + ok(hr == S_OK, "got %#lx\n", hr); + + /* At least the second item will be delayed until after shutdown. Execution still occurs after shutdown. */ + hr = RtwqShutdown(); + ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); + + res = wait_async_callback_result(&test_callback->IRtwqAsyncCallback_iface, 100, &callback_result); + flaky_wine + ok(res == 0, "got %#lx\n", res); + res = wait_async_callback_result(&test_callback2->IRtwqAsyncCallback_iface, 100, &callback_result); + todo_wine + ok(res == 0, "got %#lx\n", res); + + refcount = IRtwqAsyncResult_Release(result); + flaky_wine + ok(!refcount, "Unexpected refcount %ld.\n", refcount); + refcount = IRtwqAsyncResult_Release(result2); + todo_wine + ok(!refcount, "Unexpected refcount %ld.\n", refcount); + + hr = RtwqStartup(); + ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); + + hr = RtwqAllocateWorkQueue(RTWQ_STANDARD_WORKQUEUE, &queue); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = RtwqCreateAsyncResult(NULL, &test_callback->IRtwqAsyncCallback_iface, NULL, &result); + ok(hr == S_OK, "Failed to create result, hr %#lx.\n", hr); + hr = RtwqCreateAsyncResult(NULL, &test_callback2->IRtwqAsyncCallback_iface, NULL, &result2); + ok(hr == S_OK, "Failed to create result, hr %#lx.\n", hr); + hr = RtwqPutWorkItem(queue, 0, result); + ok(hr == S_OK, "got %#lx\n", hr); + hr = RtwqPutWorkItem(queue, 0, result2); + ok(hr == S_OK, "got %#lx\n", hr); + + /* Execution still occurs after unlock. */ + RtwqUnlockWorkQueue(queue); + + res = wait_async_callback_result(&test_callback->IRtwqAsyncCallback_iface, 100, &callback_result); + flaky_wine + ok(res == 0, "got %#lx\n", res); + res = wait_async_callback_result(&test_callback2->IRtwqAsyncCallback_iface, 100, &callback_result); + todo_wine + ok(res == 0, "got %#lx\n", res); + + refcount = IRtwqAsyncResult_Release(result); + flaky_wine + ok(!refcount, "Unexpected refcount %ld.\n", refcount); + /* An internal reference to result2 may still be held here even in Windows. */ + IRtwqAsyncResult_Release(result2); + + hr = RtwqShutdown(); + ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); + + IRtwqAsyncCallback_Release(&test_callback->IRtwqAsyncCallback_iface); + IRtwqAsyncCallback_Release(&test_callback2->IRtwqAsyncCallback_iface); +} + START_TEST(rtworkq) { test_platform_init(); test_work_queue(); test_scheduled_items(); + test_queue_shutdown(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7174
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- dlls/mfplat/tests/mfplat.c | 1 - dlls/rtworkq/queue.c | 2 +- dlls/rtworkq/tests/rtworkq.c | 8 -------- 3 files changed, 1 insertion(+), 10 deletions(-) diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index d75849a23b3..53058082964 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -4425,7 +4425,6 @@ static void test_scheduled_items(void) /* A callback invocation with RTWQ_E_OPERATION_CANCELLED may have been pending * when MFShutdown() was called. Release depends upon its execution. */ refcount = IMFAsyncResult_Release(result); - flaky_wine ok(refcount == 0, "Unexpected refcount %lu.\n", refcount); IMFAsyncCallback_Release(&callback->IMFAsyncCallback_iface); diff --git a/dlls/rtworkq/queue.c b/dlls/rtworkq/queue.c index 84ea084e2fa..644a29c04a5 100644 --- a/dlls/rtworkq/queue.c +++ b/dlls/rtworkq/queue.c @@ -365,7 +365,7 @@ static BOOL pool_queue_shutdown(struct queue *queue) if (!queue->pool) return FALSE; - CloseThreadpoolCleanupGroupMembers(queue->envs[0].CleanupGroup, TRUE, NULL); + CloseThreadpoolCleanupGroupMembers(queue->envs[0].CleanupGroup, FALSE, NULL); CloseThreadpool(queue->pool); queue->pool = NULL; diff --git a/dlls/rtworkq/tests/rtworkq.c b/dlls/rtworkq/tests/rtworkq.c index d938be3dd76..3ea66132145 100644 --- a/dlls/rtworkq/tests/rtworkq.c +++ b/dlls/rtworkq/tests/rtworkq.c @@ -481,7 +481,6 @@ static void test_scheduled_items(void) /* One or two callback invocations with RTWQ_E_OPERATION_CANCELLED may have been * pending when RtwqShutdown() was called. Release depends upon their execution. */ refcount = IRtwqAsyncResult_Release(result); - flaky_wine ok(refcount == 0, "Unexpected refcount %lu.\n", refcount); IRtwqAsyncCallback_Release(&test_callback->IRtwqAsyncCallback_iface); @@ -517,17 +516,13 @@ static void test_queue_shutdown(void) ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); res = wait_async_callback_result(&test_callback->IRtwqAsyncCallback_iface, 100, &callback_result); - flaky_wine ok(res == 0, "got %#lx\n", res); res = wait_async_callback_result(&test_callback2->IRtwqAsyncCallback_iface, 100, &callback_result); - todo_wine ok(res == 0, "got %#lx\n", res); refcount = IRtwqAsyncResult_Release(result); - flaky_wine ok(!refcount, "Unexpected refcount %ld.\n", refcount); refcount = IRtwqAsyncResult_Release(result2); - todo_wine ok(!refcount, "Unexpected refcount %ld.\n", refcount); hr = RtwqStartup(); @@ -548,14 +543,11 @@ static void test_queue_shutdown(void) RtwqUnlockWorkQueue(queue); res = wait_async_callback_result(&test_callback->IRtwqAsyncCallback_iface, 100, &callback_result); - flaky_wine ok(res == 0, "got %#lx\n", res); res = wait_async_callback_result(&test_callback2->IRtwqAsyncCallback_iface, 100, &callback_result); - todo_wine ok(res == 0, "got %#lx\n", res); refcount = IRtwqAsyncResult_Release(result); - flaky_wine ok(!refcount, "Unexpected refcount %ld.\n", refcount); /* An internal reference to result2 may still be held here even in Windows. */ IRtwqAsyncResult_Release(result2); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7174
All commits from !7244 are here now. I added an rtworkq version of test_scheduled_items(). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7174#note_95110
Nikolay Sivov (@nsivov) commented about dlls/mfplat/main.c:
hr = IMFSchemeHandler_BeginCreateObject(handler, url, flags, props, cancel_cookie ? &inner_cookie : NULL, (IMFAsyncCallback *)&resolver->url_callback, (IUnknown *)result);
- if (SUCCEEDED(hr) && inner_cookie) - resolver_create_cancel_object((IUnknown *)handler, OBJECT_FROM_URL, inner_cookie, cancel_cookie); + if (inner_cookie) + { + if (SUCCEEDED(hr)) + resolver_create_cancel_object((IUnknown *)handler, OBJECT_FROM_URL, inner_cookie, cancel_cookie); + IUnknown_Release(inner_cookie); + }
Does this mean that failed BeginCreateObject() call still returns valid object? I think that's unexpected in general. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7174#note_95334
On Thu Feb 20 12:20:42 2025 +0000, Nikolay Sivov wrote:
Does this mean that failed BeginCreateObject() call still returns valid object? I think that's unexpected in general. The series seems fine otherwise.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7174#note_95335
participants (3)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Nikolay Sivov (＠nsivov)