[PATCH v2 0/2] MR8697: combase: Don't marshal objects marked as IAgileObject in RoGetAgileReference.
[Chromium obtains an agile reference](https://source.chromium.org/chromium/chromium/src/+/main:device/bluetooth/bl...) to WinRT activation factories with RoAgileReference. As we currently do not support marshaling WinRT objects, this fails. However, as objects marked with IAgileObject don't need to be marshaled across apartments, we can trivially implement `RoAgileReference` for such objects, as most activation factories tend to be agile. -- v2: combase: Don't marshal objects that implement IAgileObject in RoGetAgileReference. https://gitlab.winehq.org/wine/wine/-/merge_requests/8697
From: Vibhav Pant <vibhavp(a)gmail.com> Objects that implement IAgileObject don't need to be marshaled across apartment boundaries. --- dlls/combase/tests/roapi.c | 64 +++++++++++++++++++++++++++++++++++++- 1 file changed, 63 insertions(+), 1 deletion(-) diff --git a/dlls/combase/tests/roapi.c b/dlls/combase/tests/roapi.c index ab5dbff6017..fb40dc60f5a 100644 --- a/dlls/combase/tests/roapi.c +++ b/dlls/combase/tests/roapi.c @@ -374,6 +374,19 @@ static HRESULT WINAPI unk_no_marshal_QueryInterface(IUnknown *iface, REFIID riid return E_NOINTERFACE; } +static HRESULT WINAPI unk_agile_QueryInterface(IUnknown *iface, REFIID riid, void **ppv) +{ + if (IsEqualGUID(riid, &IID_IUnknown) + || IsEqualGUID(riid, &IID_IAgileObject)) + { + *ppv = iface; + IUnknown_AddRef(iface); + return S_OK; + } + + return E_NOINTERFACE; +} + static ULONG WINAPI unk_AddRef(IUnknown *iface) { struct unk_impl *impl = impl_from_IUnknown(iface); @@ -400,6 +413,13 @@ static const IUnknownVtbl unk_no_marshal_vtbl = unk_Release }; +static const IUnknownVtbl unk_agile_vtbl = +{ + unk_agile_QueryInterface, + unk_AddRef, + unk_Release +}; + struct test_RoGetAgileReference_thread_param { enum AgileReferenceOptions option; @@ -407,6 +427,7 @@ struct test_RoGetAgileReference_thread_param RO_INIT_TYPE to_type; IAgileReference *agile_reference; IUnknown *unk_obj; + BOOLEAN obj_is_agile; }; static DWORD CALLBACK test_RoGetAgileReference_thread_proc(void *arg) @@ -423,7 +444,14 @@ static DWORD CALLBACK test_RoGetAgileReference_thread_proc(void *arg) hr = IAgileReference_Resolve(param->agile_reference, &IID_IUnknown, (void **)&unknown); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); ok(!!unknown, "Expected pointer not NULL.\n"); - if (param->from_type == RO_INIT_MULTITHREADED && param->to_type == RO_INIT_MULTITHREADED) + if (param->obj_is_agile) + { + todo_wine_if(param->from_type == RO_INIT_SINGLETHREADED || param->to_type == RO_INIT_SINGLETHREADED) + ok(unknown == param->unk_obj, "Expected the same object.\n"); + todo_wine_if(param->from_type == RO_INIT_MULTITHREADED && param->to_type == RO_INIT_MULTITHREADED) + EXPECT_REF(param->unk_obj, 4); + } + else if (param->from_type == RO_INIT_MULTITHREADED && param->to_type == RO_INIT_MULTITHREADED) { ok(unknown == param->unk_obj, "Expected the same object.\n"); todo_wine_if(param->option == AGILEREFERENCE_DEFAULT) @@ -448,6 +476,7 @@ static void test_RoGetAgileReference(void) struct test_RoGetAgileReference_thread_param param; struct unk_impl unk_no_marshal_obj = {{&unk_no_marshal_vtbl}, 1}; struct unk_impl unk_obj = {{&unk_vtbl}, 1}; + struct unk_impl unk_agile_obj = {{&unk_agile_vtbl}, 1}; enum AgileReferenceOptions option; IAgileReference *agile_reference; RO_INIT_TYPE from_type, to_type; @@ -521,6 +550,7 @@ static void test_RoGetAgileReference(void) param.to_type = to_type; param.agile_reference = agile_reference; param.unk_obj = &unk_obj.IUnknown_iface; + param.obj_is_agile = FALSE; thread = CreateThread(NULL, 0, test_RoGetAgileReference_thread_proc, ¶m, 0, NULL); flush_events(); ret = WaitForSingleObject(thread, 100); @@ -532,6 +562,38 @@ static void test_RoGetAgileReference(void) IAgileReference_Release(agile_reference); EXPECT_REF(&unk_obj, 1); + hr = RoGetAgileReference(option, &IID_IUnknown, &unk_agile_obj.IUnknown_iface, &agile_reference); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + ok(!!agile_reference, "Got unexpected agile_reference.\n"); + todo_wine_if(option == AGILEREFERENCE_DEFAULT) + EXPECT_REF(&unk_agile_obj, 2); + + unknown = NULL; + hr = IAgileReference_Resolve(agile_reference, &IID_IUnknown, (void **)&unknown); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + ok(!!unknown, "Expected pointer not NULL.\n"); + ok(unknown == &unk_agile_obj.IUnknown_iface, "Expected the same object.\n"); + todo_wine + EXPECT_REF(&unk_agile_obj, 3); + + for (to_type = RO_INIT_SINGLETHREADED; to_type <= RO_INIT_MULTITHREADED; to_type++) + { + param.option = option; + param.from_type = from_type; + param.to_type = to_type; + param.agile_reference = agile_reference; + param.unk_obj = &unk_agile_obj.IUnknown_iface; + param.obj_is_agile = TRUE; + thread = CreateThread(NULL, 0, test_RoGetAgileReference_thread_proc, ¶m, 0, NULL); + flush_events(); + ret = WaitForSingleObject(thread, 100); + ok(!ret, "WaitForSingleObject failed, error %ld.\n", GetLastError()); + } + + IUnknown_Release(unknown); + IAgileReference_Release(agile_reference); + EXPECT_REF(&unk_obj, 1); + RoUninitialize(); winetest_pop_context(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8697
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/combase/roapi.c | 27 +++++++++++++++++++++------ dlls/combase/tests/roapi.c | 4 ---- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/dlls/combase/roapi.c b/dlls/combase/roapi.c index df3a3c5c176..48faf62202d 100644 --- a/dlls/combase/roapi.c +++ b/dlls/combase/roapi.c @@ -245,6 +245,7 @@ struct agile_reference IStream *marshal_stream; CRITICAL_SECTION cs; IUnknown *obj; + BOOLEAN is_agile; LONG ref; }; @@ -331,6 +332,9 @@ static HRESULT WINAPI agile_ref_Resolve(IAgileReference *iface, REFIID riid, voi TRACE("(%p, %s, %p)\n", iface, debugstr_guid(riid), obj); + if (impl->is_agile) + return IUnknown_QueryInterface(impl->obj, riid, obj); + EnterCriticalSection(&impl->cs); if (impl->option == AGILEREFERENCE_DELAYEDMARSHAL && impl->marshal_stream == NULL) { @@ -359,6 +363,16 @@ static const IAgileReferenceVtbl agile_ref_vtbl = agile_ref_Resolve, }; +static BOOL is_object_agile(IUnknown *obj) +{ + IAgileObject *agile; + HRESULT hr; + + hr = IUnknown_QueryInterface(obj, &IID_IAgileObject, (void **)&agile); + if (SUCCEEDED(hr)) IUnknown_Release(agile); + return SUCCEEDED(hr); +} + /*********************************************************************** * RoGetAgileReference (combase.@) */ @@ -399,8 +413,14 @@ HRESULT WINAPI RoGetAgileReference(enum AgileReferenceOptions option, REFIID rii impl->IAgileReference_iface.lpVtbl = &agile_ref_vtbl; impl->option = option; impl->ref = 1; + impl->is_agile = is_object_agile(obj); - if (option == AGILEREFERENCE_DEFAULT) + if (option == AGILEREFERENCE_DELAYEDMARSHAL || impl->is_agile) + { + impl->obj = obj; + IUnknown_AddRef(impl->obj); + } + else if (option == AGILEREFERENCE_DEFAULT) { if (FAILED(hr = marshal_object_in_agile_reference(impl, riid, obj))) { @@ -408,11 +428,6 @@ HRESULT WINAPI RoGetAgileReference(enum AgileReferenceOptions option, REFIID rii return hr; } } - else if (option == AGILEREFERENCE_DELAYEDMARSHAL) - { - impl->obj = obj; - IUnknown_AddRef(impl->obj); - } InitializeCriticalSection(&impl->cs); diff --git a/dlls/combase/tests/roapi.c b/dlls/combase/tests/roapi.c index fb40dc60f5a..91421605a75 100644 --- a/dlls/combase/tests/roapi.c +++ b/dlls/combase/tests/roapi.c @@ -446,9 +446,7 @@ static DWORD CALLBACK test_RoGetAgileReference_thread_proc(void *arg) ok(!!unknown, "Expected pointer not NULL.\n"); if (param->obj_is_agile) { - todo_wine_if(param->from_type == RO_INIT_SINGLETHREADED || param->to_type == RO_INIT_SINGLETHREADED) ok(unknown == param->unk_obj, "Expected the same object.\n"); - todo_wine_if(param->from_type == RO_INIT_MULTITHREADED && param->to_type == RO_INIT_MULTITHREADED) EXPECT_REF(param->unk_obj, 4); } else if (param->from_type == RO_INIT_MULTITHREADED && param->to_type == RO_INIT_MULTITHREADED) @@ -565,7 +563,6 @@ static void test_RoGetAgileReference(void) hr = RoGetAgileReference(option, &IID_IUnknown, &unk_agile_obj.IUnknown_iface, &agile_reference); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); ok(!!agile_reference, "Got unexpected agile_reference.\n"); - todo_wine_if(option == AGILEREFERENCE_DEFAULT) EXPECT_REF(&unk_agile_obj, 2); unknown = NULL; @@ -573,7 +570,6 @@ static void test_RoGetAgileReference(void) ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); ok(!!unknown, "Expected pointer not NULL.\n"); ok(unknown == &unk_agile_obj.IUnknown_iface, "Expected the same object.\n"); - todo_wine EXPECT_REF(&unk_agile_obj, 3); for (to_type = RO_INIT_SINGLETHREADED; to_type <= RO_INIT_MULTITHREADED; to_type++) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8697
On Tue Aug 26 12:50:11 2025 +0000, Vibhav Pant wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/8697/diffs?diff_id=203241&start_sha=b93274616a726cd59ca056b6c5260910925ff715#fc21ec7d10383846bd168b18f7fe3582dae28d3f_607_593) Fair, WinRT marshaling is a separate problem anyway. Removed this commit in the latest revision.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8697#note_113886
On Sat Aug 23 16:36:05 2025 +0000, Nikolay Sivov wrote:
I would rather combine this with AGILEREFERENCE_DELAYEDMARSHAL block, using additional helper like "bool is_object_agile()". Right now you need to look twice to see why "unknown" is not released. Makes sense. I have simplified this a bit more, by factoring all the interface checks into a generic `object_has_interface` helper, which removes the need for a separate `unknown` variable altogether.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8697#note_113887
participants (2)
-
Vibhav Pant -
Vibhav Pant (＠vibhavp)