[PATCH v3 0/2] MR11018: combase: Implement delayed marshaling for RoGetAgileReference
When RoGetAgileReference is used with AGILEREFERENCE_DELAYEDMARSHAL, the returned IAgileReference also contains a reference to the apartment/context the object belongs to. Thus, when IAgileReference::Resolve() gets called, the object will get marshaled inside the owning apartment/context, not the caller's apartment. -- v3: combase: Implement delayed marshaling for RoGetAgileReference. combase/tests: Add tests for delayed marshaling of objects with apartment affinity. https://gitlab.winehq.org/wine/wine/-/merge_requests/11018
From: Vibhav Pant <vibhavp@gmail.com> When RoGetAgileReference is used with AGILEREFERENCE_DELAYEDMARSHAL, the returned IAgileReference also contains a reference to the apartment/context the object belongs to. Thus, when IAgileReference::Resolve() gets called, the object will get marshaled inside the owning apartment/context, not the caller's apartment. --- dlls/combase/tests/roapi.c | 227 +++++++++++++++++++++++++++++++++++++ include/roapi.h | 1 + 2 files changed, 228 insertions(+) diff --git a/dlls/combase/tests/roapi.c b/dlls/combase/tests/roapi.c index e4100cb11bc..76cb2536482 100644 --- a/dlls/combase/tests/roapi.c +++ b/dlls/combase/tests/roapi.c @@ -450,6 +450,166 @@ static const IUnknownVtbl unk_agile_vtbl = unk_Release }; +/* IUnknown implementation that has affinity to the STA it was created in. */ +struct unk_ctx_impl +{ + IUnknown IUnknown_iface; + BOOL todo; + UINT64 apt_id; /* Identifier of the apartment this object belongs to. */ + ULONG_PTR context; /* The COM context this object belongs to. */ + APTTYPE type; /* APTTYPE of the apartment this object belongs to. */ + DWORD thread_id; + GUID com_thread_id; + LONG ref; +}; + +static inline struct unk_ctx_impl *impl_unk_ctx_impl_from_IUnknown(IUnknown *iface) +{ + return CONTAINING_RECORD(iface, struct unk_ctx_impl, IUnknown_iface); +} + +/* If true, the method call is being made from the test thread. */ +static BOOL caller_test_thread; + +#define test_apartment_context(impl) test_apartment_context_(__LINE__, impl) +static void test_apartment_context_(int line, const struct unk_ctx_impl *impl) +{ + APTTYPEQUALIFIER qualifier; + ULONG_PTR cur_ctx; + UINT64 cur_id; + APTTYPE type; + HRESULT hr; + + hr = CoGetContextToken(&cur_ctx); + ok_(__FILE__, line)(hr == S_OK, "CoGetContextToken failed, got hr %#lx\n", hr); + /* As this object has apartment-affinity, its methods can only be called from the apartment and context it was + * created in. */ + todo_wine_if(impl->todo) ok_(__FILE__, line)(cur_ctx == impl->context, "got cur_ctx %#Ix != %#Ix\n", cur_ctx, impl->context); + + hr = CoGetApartmentType(&type, &qualifier); + ok_(__FILE__, line)(hr == S_OK, "CoGetApartmentType failed, got hr %#lx\n", hr); + todo_wine_if(impl->todo) ok_(__FILE__, line)(type == impl->type, "got type %d\n", type); + + hr = RoGetApartmentIdentifier(&cur_id); + /* win10 and below fail with ERROR_API_UNAVAILABLE */ + ok_(__FILE__, line)(hr == S_OK || broken(hr == HRESULT_FROM_WIN32(ERROR_API_UNAVAILABLE)), + "RoGetApartmentIdentifier failed, got hr %#lx\n", hr); + if (SUCCEEDED(hr)) + ok_(__FILE__, line)(cur_id == impl->apt_id, "got cur_id %#I64x != %#I64x\n", cur_id, impl->apt_id); + + if (caller_test_thread) + { + DWORD cur_tid = GetCurrentThreadId(); + IUnknown *unk = (IUnknown *)cur_ctx; + GUID ctx_com_tid, cur_com_tid; + IComThreadingInfo *info; + + hr = IUnknown_QueryInterface(unk, &IID_IComThreadingInfo, (void **)&info); + ok_(__FILE__, line)(hr == S_OK, "QueryInterface failed, got hr %#lx\n", hr); + + hr = IComThreadingInfo_GetCurrentLogicalThreadId(info, &ctx_com_tid); + ok_(__FILE__, line)(hr == S_OK, "GetCurrentLogicalThreadId failed, got hr %#lx\n", hr); + + hr = CoGetCurrentLogicalThreadId(&cur_com_tid); + ok_(__FILE__, line)(hr == S_OK, "CoGetCurrentLogicalThreadId failed, got hr %#lx\n", hr); + ok_(__FILE__, line)(IsEqualGUID(&cur_com_tid, &ctx_com_tid), "Got cur_com_tid %s != %s.\n", + debugstr_guid(&cur_com_tid), debugstr_guid(&ctx_com_tid)); + IComThreadingInfo_Release(info); + + /* If this object belongs to an STA, we should now be in the same thread that the object was created in. */ + if (impl->type == APTTYPE_STA || impl->type == APTTYPE_MAINSTA) + { + todo_wine ok(cur_tid == impl->thread_id, "Got cur_tid %lu != %lu\n", cur_tid, impl->thread_id); + ok(!IsEqualGUID(&ctx_com_tid, &impl->com_thread_id), "Got cur_com_tid %s != %s\n", + debugstr_guid(&ctx_com_tid), debugstr_guid(&impl->com_thread_id)); + } + else + /* If this object belongs to a MTA, then the method call will be made from the same thread as that of the + * caller. */ + ok(cur_tid != impl->thread_id, "Got cur_tid %lu\n", cur_tid); + } +} + +static HRESULT WINAPI unk_ctx_impl_QueryInterface(IUnknown *iface, const GUID *iid, void **out) +{ + struct unk_ctx_impl *impl = impl_unk_ctx_impl_from_IUnknown(iface); + + if (winetest_debug > 1) + trace("(%p, %s, %p)\n", iface, debugstr_guid(iid), out); + + test_apartment_context(impl); + if (IsEqualGUID(iid, &IID_IUnknown)) + { + *out = &impl->IUnknown_iface; + IUnknown_AddRef((IUnknown *)*out); + return S_OK; + } + + *out = NULL; + if (winetest_debug > 1) + trace("%s not implemeneted, returning E_NOINTERFACE\n", debugstr_guid(iid)); + return E_NOINTERFACE; +} + +static ULONG WINAPI unk_ctx_impl_AddRef(IUnknown *iface) +{ + struct unk_ctx_impl *impl = impl_unk_ctx_impl_from_IUnknown(iface); + + test_apartment_context(impl); + return InterlockedIncrement(&impl->ref); +} + +static ULONG WINAPI unk_ctx_impl_Release(IUnknown *iface) +{ + struct unk_ctx_impl *impl = impl_unk_ctx_impl_from_IUnknown(iface); + ULONG ref = InterlockedDecrement(&impl->ref); + + test_apartment_context(impl); + if (!ref) free(impl); + return ref; +} + +static const IUnknownVtbl unk_ctx_impl_IUnknown_vtbl = +{ + unk_ctx_impl_QueryInterface, + unk_ctx_impl_AddRef, + unk_ctx_impl_Release, +}; + +static IUnknown *unk_ctx_impl_create(void) +{ + APTTYPEQUALIFIER qualifier; + struct unk_ctx_impl *impl; + UINT64 apt_id = 0; + ULONG_PTR context; + APTTYPE type; + HRESULT hr; + GUID id; + + hr = CoGetApartmentType(&type, &qualifier); + ok(hr == S_OK, "got hr %#lx\n", hr); + + hr = CoGetContextToken(&context); + ok(hr == S_OK, "got hr %#lx\n", hr); + + hr = RoGetApartmentIdentifier(&apt_id); + /* win10 and below fail with ERROR_API_UNAVAILABLE */ + ok(hr == S_OK || broken(hr == HRESULT_FROM_WIN32(ERROR_API_UNAVAILABLE)), "got hr %#lx\n", hr); + + hr = CoGetCurrentLogicalThreadId(&id); + ok(hr == S_OK, "got hr %#lx\n", hr); + + if (!(impl = calloc(1, sizeof(*impl)))) return NULL; + impl->IUnknown_iface.lpVtbl = &unk_ctx_impl_IUnknown_vtbl; + impl->context = context; + impl->type = type; + impl->apt_id = apt_id; + impl->com_thread_id = id; + impl->thread_id = GetCurrentThreadId(); + impl->ref = 1; + return &impl->IUnknown_iface; +} + struct test_RoGetAgileReference_thread_param { enum AgileReferenceOptions option; @@ -499,12 +659,42 @@ static DWORD CALLBACK test_RoGetAgileReference_thread_proc(void *arg) return 0; } +struct test_agile_resolve_context_params +{ + RO_INIT_TYPE from_type; + RO_INIT_TYPE to_type; + IAgileReference *ref; +}; + +static DWORD CALLBACK test_agile_resolve_context(void *arg) +{ + struct test_agile_resolve_context_params *params = arg; + IUnknown *unknown; + HRESULT hr; + + caller_test_thread = TRUE; + hr = RoInitialize(params->to_type); + ok(hr == S_OK, "got hr %#lx.\n", hr); + + winetest_push_context("from_type=%d, to_type=%d", params->from_type, params->to_type); + hr = IAgileReference_Resolve(params->ref, &IID_IUnknown, (void **)&unknown); + todo_wine_if(params->to_type == RO_INIT_MULTITHREADED) ok(hr == S_OK, "got hr %#lx\n", hr); + if (SUCCEEDED(hr)) + IUnknown_Release(unknown); + winetest_pop_context(); + + RoUninitialize(); + caller_test_thread = FALSE; + return 0; +} + 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}; + struct unk_ctx_impl *unk_ctx_impl; enum AgileReferenceOptions option; IAgileReference *agile_reference; RO_INIT_TYPE from_type, to_type; @@ -624,6 +814,43 @@ static void test_RoGetAgileReference(void) winetest_pop_context(); } } + + /* Tests specific to delayed marshaling */ + for (from_type = RO_INIT_SINGLETHREADED; from_type <= RO_INIT_MULTITHREADED; from_type++) + { + winetest_push_context("from_type=%d", from_type); + hr = RoInitialize(from_type); + ok(hr == S_OK, "got hr %#lx.\n", hr); + + unknown = unk_ctx_impl_create(); + ok(unknown != NULL, "got unknown %p.\n", unknown); + + unk_ctx_impl = impl_unk_ctx_impl_from_IUnknown(unknown); + hr = RoGetAgileReference(AGILEREFERENCE_DELAYEDMARSHAL, &IID_IUnknown, unknown, &agile_reference); + ok(hr == S_OK, "got hr %#lx\n", hr); + EXPECT_REF(unknown, 2); + + for (to_type = RO_INIT_SINGLETHREADED; to_type <= RO_INIT_MULTITHREADED; to_type++) + { + struct test_agile_resolve_context_params params = {from_type, to_type, agile_reference}; + + winetest_push_context("to_type=%d", to_type); + unk_ctx_impl->todo = TRUE; + thread = CreateThread(NULL, 0, test_agile_resolve_context, ¶ms, 0, NULL); + flush_events(); + ret = WaitForSingleObject(thread, INFINITE); + ok(!ret, "got ret %lu\n", ret); + CloseHandle(thread); + unk_ctx_impl->todo = FALSE; + winetest_pop_context(); + } + + IAgileReference_Release(agile_reference); + EXPECT_REF(unknown, 1); + IUnknown_Release(unknown); + RoUninitialize(); + winetest_pop_context(); + } } static void test_RoGetErrorReportingFlags(void) diff --git a/include/roapi.h b/include/roapi.h index bcaa8e9fea0..18baf9e2017 100644 --- a/include/roapi.h +++ b/include/roapi.h @@ -46,6 +46,7 @@ HRESULT WINAPI RoActivateInstance(HSTRING classid, IInspectable **instance); HRESULT WINAPI RoGetActivationFactory(HSTRING classid, REFIID iid, void **class_factory); HRESULT WINAPI RoInitialize(RO_INIT_TYPE type); void WINAPI RoUninitialize(void); +HRESULT WINAPI RoGetApartmentIdentifier(UINT64 *identifier); #ifdef __cplusplus } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11018
From: Vibhav Pant <vibhavp@gmail.com> --- dlls/combase/roapi.c | 49 ++++++++++++++++++++++++++++++++++---- dlls/combase/tests/roapi.c | 8 +++---- 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/dlls/combase/roapi.c b/dlls/combase/roapi.c index f4f7c0aac26..6085b4897a1 100644 --- a/dlls/combase/roapi.c +++ b/dlls/combase/roapi.c @@ -18,6 +18,8 @@ */ #define COBJMACROS #include "objbase.h" +#include "ctxtcall.h" +#include "comsvcs.h" #include "initguid.h" #include "roapi.h" #include "roparameterizediid.h" @@ -246,6 +248,7 @@ struct agile_reference CRITICAL_SECTION cs; IUnknown *obj; BOOLEAN is_agile; + IUnknown *ctx; LONG ref; }; @@ -324,10 +327,28 @@ static ULONG WINAPI agile_ref_Release(IAgileReference *iface) return ref; } +struct marshal_context_params +{ + struct agile_reference *impl; + REFIID iid; +}; + +static HRESULT WINAPI marshal_object_in_context(ComCallData *arg) +{ + struct marshal_context_params *params = (struct marshal_context_params *)arg; + HRESULT hr; + + hr = marshal_object_in_agile_reference(params->impl, params->iid, params->impl->obj); + IUnknown_Release(params->impl->obj); + params->impl->obj = NULL; + return hr; +} + static HRESULT WINAPI agile_ref_Resolve(IAgileReference *iface, REFIID riid, void **obj) { struct agile_reference *impl = impl_from_IAgileReference(iface); LARGE_INTEGER zero = {0}; + void *cur_ctx; HRESULT hr; TRACE("(%p, %s, %p)\n", iface, debugstr_guid(riid), obj); @@ -335,17 +356,29 @@ static HRESULT WINAPI agile_ref_Resolve(IAgileReference *iface, REFIID riid, voi if (impl->is_agile) return IUnknown_QueryInterface(impl->obj, riid, obj); + if (FAILED(hr = CoGetContextToken((ULONG_PTR *)&cur_ctx))) + return hr; + EnterCriticalSection(&impl->cs); if (impl->option == AGILEREFERENCE_DELAYEDMARSHAL && impl->marshal_stream == NULL) { - if (FAILED(hr = marshal_object_in_agile_reference(impl, riid, impl->obj))) + struct marshal_context_params params = { impl, riid }; + IContextCallback *ctx; + + if (FAILED(hr = IUnknown_QueryInterface(impl->ctx, &IID_IContextCallback, (void **)&ctx))) { LeaveCriticalSection(&impl->cs); return hr; } - IUnknown_Release(impl->obj); - impl->obj = NULL; + hr = IContextCallback_ContextCallback(ctx, marshal_object_in_context, (ComCallData *)¶ms, + &IID_IContextCallback, 5, NULL); + IContextCallback_Release(ctx); + if (FAILED(hr)) + { + LeaveCriticalSection(&impl->cs); + return hr; + } } if (SUCCEEDED(hr = IStream_Seek(impl->marshal_stream, zero, STREAM_SEEK_SET, NULL))) @@ -380,6 +413,7 @@ static BOOL object_has_interface(IUnknown *obj, REFIID iid) HRESULT WINAPI RoGetAgileReference(enum AgileReferenceOptions option, REFIID riid, IUnknown *obj, IAgileReference **agile_reference) { + struct apartment *apt; struct agile_reference *impl; HRESULT hr; @@ -388,11 +422,13 @@ HRESULT WINAPI RoGetAgileReference(enum AgileReferenceOptions option, REFIID rii if (option != AGILEREFERENCE_DEFAULT && option != AGILEREFERENCE_DELAYEDMARSHAL) return E_INVALIDARG; - if (!InternalIsProcessInitialized()) + if (!(apt = apartment_get_current_or_mta())) { ERR("Apartment not initialized\n"); return CO_E_NOTINITIALIZED; } + rpc_start_remoting(apt); + apartment_release(apt); if (!object_has_interface(obj, riid)) return E_NOINTERFACE; @@ -407,6 +443,11 @@ HRESULT WINAPI RoGetAgileReference(enum AgileReferenceOptions option, REFIID rii impl->option = option; impl->is_agile = object_has_interface(obj, &IID_IAgileObject); impl->ref = 1; + if (FAILED(hr = CoGetContextToken((ULONG_PTR *)&impl->ctx))) + { + free( impl ); + return hr; + } if (option == AGILEREFERENCE_DELAYEDMARSHAL || impl->is_agile) { diff --git a/dlls/combase/tests/roapi.c b/dlls/combase/tests/roapi.c index 76cb2536482..70977c9920e 100644 --- a/dlls/combase/tests/roapi.c +++ b/dlls/combase/tests/roapi.c @@ -484,11 +484,11 @@ static void test_apartment_context_(int line, const struct unk_ctx_impl *impl) ok_(__FILE__, line)(hr == S_OK, "CoGetContextToken failed, got hr %#lx\n", hr); /* As this object has apartment-affinity, its methods can only be called from the apartment and context it was * created in. */ - todo_wine_if(impl->todo) ok_(__FILE__, line)(cur_ctx == impl->context, "got cur_ctx %#Ix != %#Ix\n", cur_ctx, impl->context); + ok_(__FILE__, line)(cur_ctx == impl->context, "got cur_ctx %#Ix != %#Ix\n", cur_ctx, impl->context); hr = CoGetApartmentType(&type, &qualifier); ok_(__FILE__, line)(hr == S_OK, "CoGetApartmentType failed, got hr %#lx\n", hr); - todo_wine_if(impl->todo) ok_(__FILE__, line)(type == impl->type, "got type %d\n", type); + ok_(__FILE__, line)(type == impl->type, "got type %d\n", type); hr = RoGetApartmentIdentifier(&cur_id); /* win10 and below fail with ERROR_API_UNAVAILABLE */ @@ -519,7 +519,7 @@ static void test_apartment_context_(int line, const struct unk_ctx_impl *impl) /* If this object belongs to an STA, we should now be in the same thread that the object was created in. */ if (impl->type == APTTYPE_STA || impl->type == APTTYPE_MAINSTA) { - todo_wine ok(cur_tid == impl->thread_id, "Got cur_tid %lu != %lu\n", cur_tid, impl->thread_id); + ok(cur_tid == impl->thread_id, "Got cur_tid %lu != %lu\n", cur_tid, impl->thread_id); ok(!IsEqualGUID(&ctx_com_tid, &impl->com_thread_id), "Got cur_com_tid %s != %s\n", debugstr_guid(&ctx_com_tid), debugstr_guid(&impl->com_thread_id)); } @@ -678,7 +678,7 @@ static DWORD CALLBACK test_agile_resolve_context(void *arg) winetest_push_context("from_type=%d, to_type=%d", params->from_type, params->to_type); hr = IAgileReference_Resolve(params->ref, &IID_IUnknown, (void **)&unknown); - todo_wine_if(params->to_type == RO_INIT_MULTITHREADED) ok(hr == S_OK, "got hr %#lx\n", hr); + ok(hr == S_OK, "got hr %#lx\n", hr); if (SUCCEEDED(hr)) IUnknown_Release(unknown); winetest_pop_context(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11018
On Fri May 29 16:50:51 2026 +0000, Vibhav Pant wrote:
Fixed, thanks. I think method numbers lower than 3 likely have some kind of an `IUnknown` fastpath in native, at least according to Raymond Chen: https://devblogs.microsoft.com/oldnewthing/20191128-00/?p=103157
The interface must not be `IUnknown`, because the COM marshaler treats `IUnknown` as a special case.¹ Similarly, the method index must not be less than 3, because the first three methods on every interface come from `IUnknown`.² I've seen the blog while implementing it (it's where the iMethod=5 suggestion comes from). I was not able to find any difference in ContextCallback behavior when iMethod==2 (and the application I was implementing it for uses iMethod=2).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11018#note_141732
This merge request was approved by Piotr Caban. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11018
This merge request was approved by Huw Davies. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11018
participants (4)
-
Huw Davies (＠huw) -
Piotr Caban (＠piotr)
-
Vibhav Pant
-
Vibhav Pant (＠vibhavp)