[PATCH 0/2] MR11018: combase: Implement delayed marshaling for RoGetAgileReference

[Vibhav Pant (＠vibhavp)]

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.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/11018

[PATCH 1/2] combase/tests: Add tests for delayed marshaling of objects with apartment affinity.

[Vibhav Pant]

From: Vibhav Pant <[e-mail]>

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 | 225 +++++++++++++++++++++++++++++++++++++
 include/roapi.h            |   1 +
 2 files changed, 226 insertions(+)

diff --git a/dlls/combase/tests/roapi.c b/dlls/combase/tests/roapi.c
index e4100cb11bc..87a8eb5c50a 100644
--- a/dlls/combase/tests/roapi.c
+++ b/dlls/combase/tests/roapi.c
@@ -450,6 +450,164 @@ 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);
+    ok_(__FILE__, line)(hr == S_OK, "RoGetApartmentIdentifier failed, got hr %#lx\n", 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 HRESULT unk_ctx_impl_create(IUnknown **out)
+{
+    APTTYPEQUALIFIER qualifier;
+    struct unk_ctx_impl *impl;
+    HRESULT hr;
+
+    if (!(impl = calloc(1, sizeof(*impl)))) return E_OUTOFMEMORY;
+
+    impl->IUnknown_iface.lpVtbl = &unk_ctx_impl_IUnknown_vtbl;
+    if (FAILED(hr = CoGetContextToken(&impl->context)))
+    {
+        free(impl);
+        return hr;
+    }
+    if (FAILED(hr = CoGetApartmentType(&impl->type, &qualifier)))
+    {
+        free(impl);
+        return hr;
+    }
+    if (FAILED(hr = RoGetApartmentIdentifier(&impl->apt_id)))
+    {
+        free(impl);
+        return hr;
+    }
+    if (FAILED(hr = CoGetCurrentLogicalThreadId(&impl->com_thread_id)))
+    {
+        free(impl);
+        return hr;
+    }
+    impl->thread_id = GetCurrentThreadId();
+    impl->ref = 1;
+    *out = &impl->IUnknown_iface;
+    return S_OK;
+}
+
 struct test_RoGetAgileReference_thread_param
 {
     enum AgileReferenceOptions option;
@@ -499,12 +657,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 +812,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);
+
+        hr = unk_ctx_impl_create(&unknown);
+        ok(hr == S_OK, "got hr %#lx\n", hr);
+
+        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

[PATCH 2/2] combase: Implement delayed marshaling for RoGetAgileReference.

[Vibhav Pant]

From: Vibhav Pant <[e-mail]>

---
 dlls/combase/roapi.c       | 60 +++++++++++++++++++++++++++++++++++---
 dlls/combase/tests/roapi.c |  8 ++---
 2 files changed, 60 insertions(+), 8 deletions(-)

diff --git a/dlls/combase/roapi.c b/dlls/combase/roapi.c
index f4f7c0aac26..00b028a8996 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,27 @@ 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);
+    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,16 +355,40 @@ 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)))
+        if (cur_ctx != impl->ctx)
         {
+            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;
+            }
+
+            hr = IContextCallback_ContextCallback(ctx, marshal_object_in_context, (ComCallData *)¶ms,
+                                                  &IID_IContextCallback, 2, NULL);
+            IContextCallback_Release(ctx);
+            if (FAILED(hr))
+            {
+                LeaveCriticalSection(&impl->cs);
+                return hr;
+            }
+        }
+        else if (FAILED(hr = marshal_object_in_agile_reference(impl, riid, impl->obj)))
+        {
+
             LeaveCriticalSection(&impl->cs);
             return hr;
         }
-
-        IUnknown_Release(impl->obj);
+        else
+            IUnknown_Release(impl->obj);
         impl->obj = NULL;
     }
 
@@ -380,6 +424,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 +433,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 +454,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 87a8eb5c50a..c004d0c69e4 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);
     ok_(__FILE__, line)(hr == S_OK, "RoGetApartmentIdentifier failed, got hr %#lx\n", hr);
@@ -516,7 +516,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));
         }
@@ -676,7 +676,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

[Piotr Caban (＠piotr)]

Piotr Caban (@piotr) commented about dlls/combase/roapi.c:
> 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)))
> +        if (cur_ctx != impl->ctx)

Is there any reason for doing this check instead of always going through ContextCallback path? There's similar optimization in ContextCallback so there should be no visible performance impact.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/11018#note_141572

[Piotr Caban (＠piotr)]

Piotr Caban (@piotr) commented about dlls/combase/roapi.c:
> if (impl->option == AGILEREFERENCE_DELAYEDMARSHAL && impl->marshal_stream == NULL)
>      {
> -        if (FAILED(hr = marshal_object_in_agile_reference(impl, riid, impl->obj)))
> +        if (cur_ctx != impl->ctx)
>          {
> +            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;
> +            }
> +
> +            hr = IContextCallback_ContextCallback(ctx, marshal_object_in_context, (ComCallData *)¶ms,
> +                                                  &IID_IContextCallback, 2, NULL);

Could you please change iMethod parameter to 5? Documentation states that values lower than 3 have special meaning (it's probably not true).

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/11018#note_141573

[Vibhav Pant (＠vibhavp)]

On Thu May 28 21:19:15 2026 +0000, Piotr Caban wrote:
> Is there any reason for doing this check instead of always going through
> ContextCallback path? There's similar optimization in ContextCallback so
> there should be no visible performance impact.
> Yes, it shouldn't be needed. Removed this in v2, thanks.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/11018#note_141706