Goes atop !7174
-- v2: rtworkq: Do not cancel pending callbacks when closing a thread pool. rtworkq/tests: Test queue shutdown with pending items.
From: Conor McCarthy cmccarthy@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);
From: Conor McCarthy cmccarthy@codeweavers.com
--- dlls/mfplat/tests/Makefile.in | 2 +- dlls/mfplat/tests/mfplat.c | 57 +++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/dlls/mfplat/tests/Makefile.in b/dlls/mfplat/tests/Makefile.in index 217ea437b3c..05c14288769 100644 --- a/dlls/mfplat/tests/Makefile.in +++ b/dlls/mfplat/tests/Makefile.in @@ -1,5 +1,5 @@ TESTDLL = mfplat.dll -IMPORTS = ole32 mfplat user32 d3d9 dxva2 mfuuid propsys uuid strmiids +IMPORTS = ole32 mfplat user32 d3d9 dxva2 mfuuid propsys uuid strmiids rtworkq
SOURCES = \ mfplat.c \ diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index df45e369928..7ed34d0305f 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"
@@ -3858,6 +3859,8 @@ static void test_MFCreateAsyncResult(void)
static void test_startup(void) { + struct test_callback *callback; + IMFAsyncResult *result; DWORD queue; HRESULT hr;
@@ -3907,6 +3910,60 @@ static void test_startup(void)
hr = MFShutdown(); ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); + + /* Rtwq equivalence */ + + hr = MFStartup(MF_VERSION, MFSTARTUP_FULL); + ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); + + hr = RtwqStartup(); + ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); + hr = RtwqShutdown(); + ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); + check_platform_lock_count(1); + + /* Matching MFStartup() with RtwqShutdown() causes shutdown. */ + hr = RtwqShutdown(); + 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 = RtwqStartup(); + 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 = RtwqUnlockPlatform(); + 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 = RtwqLockPlatform(); + ok(hr == S_OK, "Failed to lock platform, hr %#lx.\n", hr); + check_platform_lock_count(2); + + IMFAsyncResult_Release(result); + + hr = RtwqShutdown(); + 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)
From: Conor McCarthy cmccarthy@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 7ed34d0305f..79525b31dec 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -3966,6 +3966,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; @@ -13333,6 +13526,7 @@ START_TEST(mfplat) CoInitialize(NULL);
test_startup(); + test_startup_counts(); test_register(); test_media_type(); test_MFCreateMediaEvent();
From: Conor McCarthy cmccarthy@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(); }
From: Conor McCarthy cmccarthy@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 79525b31dec..8ced5518c5f 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -3978,7 +3978,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); @@ -4001,7 +4000,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); @@ -4052,7 +4050,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. */ @@ -4136,11 +4133,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); @@ -4150,7 +4145,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);
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 | 81 +++++++++++++++++++++++++++++++++--- dlls/rtworkq/tests/rtworkq.c | 8 ---- 3 files changed, 75 insertions(+), 21 deletions(-)
diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index 8ced5518c5f..be288a30e10 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -4017,13 +4017,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);
@@ -4035,14 +4033,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(); @@ -4104,7 +4100,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); @@ -4121,12 +4116,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..394b2b30abf 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,58 @@ static struct async_result *impl_from_IRtwqAsyncResult(IRtwqAsyncResult *iface) return CONTAINING_RECORD(iface, struct async_result, result.AsyncResult); }
+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; + + if (list_empty(&async_result_cache)) + return NULL; + + 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(cur); + RtwqUnlockPlatform(); + } + + 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 +1110,12 @@ 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(result); + RtwqUnlockPlatform(); + } }
return refcount; @@ -1133,10 +1199,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 +1310,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();
From: Conor McCarthy cmccarthy@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);
From: Conor McCarthy cmccarthy@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 be288a30e10..9a00369870f 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -4649,6 +4649,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);
From: Conor McCarthy cmccarthy@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 9a00369870f..99fe44ca98b 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -4393,8 +4393,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);
@@ -4404,6 +4402,13 @@ static void test_scheduled_items(void) hr = MFShutdown(); ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr);
+ Sleep(20); + /* An 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); }
From: Conor McCarthy cmccarthy@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 05e81a3116d..6404ffeda87 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);
@@ -425,8 +429,89 @@ static void test_work_queue(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_queue_shutdown(); }
From: Conor McCarthy cmccarthy@codeweavers.com
--- dlls/mfplat/tests/mfplat.c | 1 - dlls/rtworkq/queue.c | 2 +- dlls/rtworkq/tests/rtworkq.c | 7 ------- 3 files changed, 1 insertion(+), 9 deletions(-)
diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index 99fe44ca98b..c68997b22f0 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -4406,7 +4406,6 @@ static void test_scheduled_items(void) /* An 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 394b2b30abf..41c0d5aa520 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 6404ffeda87..5814a17e540 100644 --- a/dlls/rtworkq/tests/rtworkq.c +++ b/dlls/rtworkq/tests/rtworkq.c @@ -459,17 +459,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(); @@ -490,14 +486,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);
Hi,
It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated.
The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details:
The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=151200
Your paranoid android.
=== w8 (32 bit report) ===
mfplat: mfplat.c:4020: Test failed: Unexpected lock count 1. mfplat.c:4025: Test failed: Unexpected lock count 2. mfplat.c:4035: Test failed: Unexpected refcount 1. mfplat.c:4036: Test failed: Unexpected lock count 2. mfplat.c:4042: Test failed: Unexpected lock count 3. mfplat.c:4103: Test failed: Unexpected lock count 1. mfplat.c:4119: Test failed: Unexpected lock count 1. mfplat.c:4123: Test failed: Unexpected lock count 2. mfplat.c:4141: Test failed: Unexpected lock count 1.
=== w8adm (32 bit report) ===
mfplat: mfplat.c:4020: Test failed: Unexpected lock count 1. mfplat.c:4025: Test failed: Unexpected lock count 2. mfplat.c:4036: Test failed: Unexpected lock count 1. mfplat.c:4042: Test failed: Unexpected lock count 3. mfplat.c:4103: Test failed: Unexpected lock count 1. mfplat.c:4119: Test failed: Unexpected lock count 1. mfplat.c:4123: Test failed: Unexpected lock count 2. mfplat.c:4141: Test failed: Unexpected lock count 1.
=== w864 (32 bit report) ===
mfplat: mfplat.c:4020: Test failed: Unexpected lock count 1. mfplat.c:4025: Test failed: Unexpected lock count 2. mfplat.c:4036: Test failed: Unexpected lock count 1. mfplat.c:4042: Test failed: Unexpected lock count 3. mfplat.c:4103: Test failed: Unexpected lock count 1. mfplat.c:4119: Test failed: Unexpected lock count 1. mfplat.c:4123: Test failed: Unexpected lock count 2. mfplat.c:4141: Test failed: Unexpected lock count 1.
=== w1064v1507 (32 bit report) ===
mfplat: mfplat.c:4020: Test failed: Unexpected lock count 1. mfplat.c:4025: Test failed: Unexpected lock count 2. mfplat.c:4036: Test failed: Unexpected lock count 1. mfplat.c:4042: Test failed: Unexpected lock count 3. mfplat.c:4103: Test failed: Unexpected lock count 1. mfplat.c:4119: Test failed: Unexpected lock count 1. mfplat.c:4123: Test failed: Unexpected lock count 2.
=== w1064v1809 (32 bit report) ===
mfplat: mfplat.c:4035: Test failed: Unexpected refcount 1.
=== w864 (64 bit report) ===
mfplat: mfplat.c:4020: Test failed: Unexpected lock count 1. mfplat.c:4025: Test failed: Unexpected lock count 2. mfplat.c:4035: Test failed: Unexpected refcount 1. mfplat.c:4036: Test failed: Unexpected lock count 2. mfplat.c:4042: Test failed: Unexpected lock count 3. mfplat.c:4103: Test failed: Unexpected lock count 1. mfplat.c:4119: Test failed: Unexpected lock count 1. mfplat.c:4123: Test failed: Unexpected lock count 2. mfplat.c:4141: Test failed: Unexpected lock count 1.
=== w1064v1507 (64 bit report) ===
mfplat: mfplat.c:4020: Test failed: Unexpected lock count 1. mfplat.c:4025: Test failed: Unexpected lock count 2. mfplat.c:4035: Test failed: Unexpected refcount 1. mfplat.c:4036: Test failed: Unexpected lock count 1. mfplat.c:4042: Test failed: Unexpected lock count 3. mfplat.c:4103: Test failed: Unexpected lock count 1. mfplat.c:4119: Test failed: Unexpected lock count 1. mfplat.c:4123: Test failed: Unexpected lock count 2.
=== w1064_tsign (64 bit report) ===
mfplat: mfplat.c:1358: Test failed: Unexpected refcount 3
=== w11pro64_amd (64 bit report) ===
mfplat: mfplat.c:4035: Test failed: Unexpected refcount 1.
=== w8 (32 bit report) ===
rtworkq: rtworkq.c:303: Test failed: Unexpected lock count 1. rtworkq.c:308: Test failed: Unexpected lock count 2. rtworkq.c:318: Test failed: Unexpected lock count 1. rtworkq.c:324: Test failed: Unexpected lock count 3. rtworkq.c:385: Test failed: Unexpected lock count 1. rtworkq.c:405: Test failed: Unexpected lock count 1. rtworkq.c:411: Test failed: Unexpected lock count 2. rtworkq.c:417: Test failed: Unexpected refcount 0.
=== w8adm (32 bit report) ===
rtworkq: rtworkq.c:303: Test failed: Unexpected lock count 1. rtworkq.c:308: Test failed: Unexpected lock count 2. rtworkq.c:318: Test failed: Unexpected lock count 1. rtworkq.c:324: Test failed: Unexpected lock count 3. rtworkq.c:385: Test failed: Unexpected lock count 1. rtworkq.c:405: Test failed: Unexpected lock count 1. rtworkq.c:411: Test failed: Unexpected lock count 2. rtworkq.c:417: Test failed: Unexpected refcount 0.
=== w864 (32 bit report) ===
rtworkq: rtworkq.c:303: Test failed: Unexpected lock count 1. rtworkq.c:308: Test failed: Unexpected lock count 2. rtworkq.c:317: Test failed: Unexpected refcount 1. rtworkq.c:318: Test failed: Unexpected lock count 2. rtworkq.c:324: Test failed: Unexpected lock count 4. rtworkq.c:385: Test failed: Unexpected lock count 1. rtworkq.c:405: Test failed: Unexpected lock count 1. rtworkq.c:411: Test failed: Unexpected lock count 2. rtworkq.c:417: Test failed: Unexpected refcount 0.
=== w1064v1507 (32 bit report) ===
rtworkq: rtworkq.c:303: Test failed: Unexpected lock count 1. rtworkq.c:308: Test failed: Unexpected lock count 2. rtworkq.c:317: Test failed: Unexpected refcount 1. rtworkq.c:318: Test failed: Unexpected lock count 1. rtworkq.c:324: Test failed: Unexpected lock count 3. rtworkq.c:385: Test failed: Unexpected lock count 1. rtworkq.c:405: Test failed: Unexpected lock count 1. rtworkq.c:411: Test failed: Unexpected lock count 2.
=== w10pro64_en_AE_u8 (32 bit report) ===
rtworkq: rtworkq.c:317: Test failed: Unexpected refcount 1.
=== w864 (64 bit report) ===
rtworkq: rtworkq.c:303: Test failed: Unexpected lock count 1. rtworkq.c:308: Test failed: Unexpected lock count 2. rtworkq.c:318: Test failed: Unexpected lock count 1. rtworkq.c:324: Test failed: Unexpected lock count 3. rtworkq.c:385: Test failed: Unexpected lock count 1. rtworkq.c:405: Test failed: Unexpected lock count 1. rtworkq.c:411: Test failed: Unexpected lock count 2. rtworkq.c:417: Test failed: Unexpected refcount 0.
=== w1064v1507 (64 bit report) ===
rtworkq: rtworkq.c:303: Test failed: Unexpected lock count 1. rtworkq.c:308: Test failed: Unexpected lock count 2. rtworkq.c:318: Test failed: Unexpected lock count 1. rtworkq.c:324: Test failed: Unexpected lock count 3. rtworkq.c:385: Test failed: Unexpected lock count 1. rtworkq.c:405: Test failed: Unexpected lock count 1. rtworkq.c:411: Test failed: Unexpected lock count 2.
=== debian11 (32 bit report) ===
Report validation errors: mfplat:mfplat crashed (c0000028)
=== debian11b (32 bit WoW report) ===
Report validation errors: mfplat:mfplat crashed (c0000028)
Nikolay Sivov (@nsivov) commented about dlls/rtworkq/queue.c:
+}
+static struct async_result *async_result_cache_pop(void) +{
- struct async_result *result;
- struct list *head;
- if (list_empty(&async_result_cache))
return NULL;- EnterCriticalSection(&async_result_cache_section);
- if ((head = list_head(&async_result_cache)))
list_remove(head);- LeaveCriticalSection(&async_result_cache_section);
Do we need the empty check? Especially outside of the lock.
Nikolay Sivov (@nsivov) commented about dlls/rtworkq/queue.c:
IUnknown_Release(result->state); if (result->result.hEvent) CloseHandle(result->result.hEvent);
free(result);RtwqUnlockPlatform();
if (!async_result_cache_push(result)){free(result);RtwqUnlockPlatform();}
This free-unlock pattern is used at least twice, I think a helper would be better. Like free_async_result() or similar.
Nikolay Sivov (@nsivov) commented about dlls/rtworkq/queue.c:
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();- }
Why are we caching this at all? If it's to keep object alive for longer then we don't need to push them back to the list on release, if it's to have LockPlatform/UnlockPlatform happening exactly once - lock when first result object is created and unlock only on shutdown? Or is additional goal to avoid frequent heap allocations?
Queue api from mfplat forwards directly to rtworkq, so would it be useful to duplicate some of these new tests entirely in rtworkq too?
There is also IRtwqPlatformEvents, maybe that one could aid in testing too.
Queue api from mfplat forwards directly to rtworkq, so would it be useful to duplicate some of these new tests entirely in rtworkq too?
The tests which had memory leaks?
There is also IRtwqPlatformEvents, maybe that one could aid in testing too.
This didn't prove particularly useful since it only notifies on shutdown, not on platform unlock. It could be used to check for shutdown, but it is currently unimplemented in Wine, and checking for shutdown with AllocateWorkQueue() is simple if not elegant.
On Thu Feb 6 10:01:39 2025 +0000, Nikolay Sivov wrote:
Why are we caching this at all? If it's to keep object alive for longer then we don't need to push them back to the list on release, if it's to have LockPlatform/UnlockPlatform happening exactly once - lock when first result object is created and unlock only on shutdown? Or is additional goal to avoid frequent heap allocations?
I added a comment on this. It's probably to avoid frequent heap allocations.
On Thu Feb 6 10:01:38 2025 +0000, Nikolay Sivov wrote:
This free-unlock pattern is used at least twice, I think a helper would be better. Like free_async_result() or similar.
Done.
On Thu Feb 6 10:01:38 2025 +0000, Nikolay Sivov wrote:
Do we need the empty check? Especially outside of the lock.
Removed.
Closing this since everything is now in !7174.
This merge request was closed by Conor McCarthy.
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Marvin -
Nikolay Sivov (＠nsivov)