[PATCH v2 0/2] MR3198: combase: Pass the correct destination context when unmarshaling using the standard marshaler.
This fixes an issue I ran into in UI Automation using an interface proxy marshaled with `MSHCTX_INPROC`. `CoUnmarshalInterface` always passes `MSHCTX_LOCAL` when the standard marshaler, regardless of what was passed to `CoMarshalInterface`. When passing an interface that uses the free threaded marshaler as an argument to a method on the proxy retrieved from `CoUnmarshalInterface`, it passes `MSHCTX_LOCAL` when trying to marshal, which the results in the free threaded marshaler trying to create a proxy/stub which fails. -- v2: combase: Use correct destination context in CoUnmarshalInterface when using the standard marshaler. https://gitlab.winehq.org/wine/wine/-/merge_requests/3198
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/ole32/tests/marshal.c | 102 +++++++++++++++++++++++++++++++++++-- 1 file changed, 97 insertions(+), 5 deletions(-) diff --git a/dlls/ole32/tests/marshal.c b/dlls/ole32/tests/marshal.c index 18f47a5c64c..0ea6148745f 100644 --- a/dlls/ole32/tests/marshal.c +++ b/dlls/ole32/tests/marshal.c @@ -351,10 +351,12 @@ static const IClassFactoryVtbl TestClassFactory_Vtbl = static IClassFactory Test_ClassFactory = { &TestClassFactory_Vtbl }; DEFINE_EXPECT(Invoke); +DEFINE_EXPECT(Connect); DEFINE_EXPECT(CreateStub); DEFINE_EXPECT(CreateProxy); DEFINE_EXPECT(GetWindow); -DEFINE_EXPECT(Disconnect); +DEFINE_EXPECT(RpcStubBuffer_Disconnect); +DEFINE_EXPECT(RpcProxyBuffer_Disconnect); static HRESULT WINAPI OleWindow_QueryInterface(IOleWindow *iface, REFIID riid, void **ppv) { @@ -476,7 +478,7 @@ static HRESULT WINAPI RpcStubBuffer_Connect(IRpcStubBuffer *iface, IUnknown *pUn static void WINAPI RpcStubBuffer_Disconnect(IRpcStubBuffer *iface) { - CHECK_EXPECT(Disconnect); + CHECK_EXPECT(RpcStubBuffer_Disconnect); } static HRESULT WINAPI RpcStubBuffer_Invoke(IRpcStubBuffer *iface, RPCOLEMESSAGE *_prpcmsg, @@ -533,6 +535,79 @@ static const IRpcStubBufferVtbl RpcStubBufferVtbl = { RpcStubBuffer_DebugServerRelease }; +typedef struct { + IRpcProxyBuffer IRpcProxyBuffer_iface; + LONG ref; + IRpcProxyBuffer *buffer; +} ProxyBufferWrapper; + +static ProxyBufferWrapper *impl_from_IRpcProxyBuffer(IRpcProxyBuffer *iface) +{ + return CONTAINING_RECORD(iface, ProxyBufferWrapper, IRpcProxyBuffer_iface); +} + +static HRESULT WINAPI RpcProxyBuffer_QueryInterface(IRpcProxyBuffer *iface, REFIID riid, void **ppv) +{ + ProxyBufferWrapper *This = impl_from_IRpcProxyBuffer(iface); + + if(IsEqualGUID(&IID_IUnknown, riid) || IsEqualGUID(&IID_IRpcProxyBuffer, riid)) { + *ppv = &This->IRpcProxyBuffer_iface; + }else { + *ppv = NULL; + return E_NOINTERFACE; + } + + IUnknown_AddRef((IUnknown*)*ppv); + return S_OK; +} + +static ULONG WINAPI RpcProxyBuffer_AddRef(IRpcProxyBuffer *iface) +{ + ProxyBufferWrapper *This = impl_from_IRpcProxyBuffer(iface); + return InterlockedIncrement(&This->ref); +} + +static ULONG WINAPI RpcProxyBuffer_Release(IRpcProxyBuffer *iface) +{ + ProxyBufferWrapper *This = impl_from_IRpcProxyBuffer(iface); + LONG ref = InterlockedDecrement(&This->ref); + if(!ref) { + IRpcProxyBuffer_Release(This->buffer); + heap_free(This); + } + return ref; +} + +static HRESULT WINAPI RpcProxyBuffer_Connect(IRpcProxyBuffer *iface, IRpcChannelBuffer *pRpcChannelBuffer) +{ + ProxyBufferWrapper *This = impl_from_IRpcProxyBuffer(iface); + void *dest_context_data; + DWORD dest_context; + HRESULT hr; + + CHECK_EXPECT(Connect); + + hr = IRpcChannelBuffer_GetDestCtx(pRpcChannelBuffer, &dest_context, &dest_context_data); + ok(hr == S_OK, "GetDestCtx failed: %08lx\n", hr); + todo_wine ok(dest_context == MSHCTX_INPROC, "desc_context = %lx\n", dest_context); + ok(!dest_context_data, "desc_context_data = %p\n", dest_context_data); + + return IRpcProxyBuffer_Connect(This->buffer, pRpcChannelBuffer); +} + +static void WINAPI RpcProxyBuffer_Disconnect(IRpcProxyBuffer *iface) +{ + CHECK_EXPECT(RpcProxyBuffer_Disconnect); +} + +static const IRpcProxyBufferVtbl RpcProxyBufferVtbl = { + RpcProxyBuffer_QueryInterface, + RpcProxyBuffer_AddRef, + RpcProxyBuffer_Release, + RpcProxyBuffer_Connect, + RpcProxyBuffer_Disconnect, +}; + static IPSFactoryBuffer *ps_factory_buffer; static HRESULT WINAPI PSFactoryBuffer_QueryInterface(IPSFactoryBuffer *iface, REFIID riid, void **ppv) @@ -561,8 +636,20 @@ static ULONG WINAPI PSFactoryBuffer_Release(IPSFactoryBuffer *iface) static HRESULT WINAPI PSFactoryBuffer_CreateProxy(IPSFactoryBuffer *iface, IUnknown *outer, REFIID riid, IRpcProxyBuffer **ppProxy, void **ppv) { + ProxyBufferWrapper *proxy; + HRESULT hr; + CHECK_EXPECT(CreateProxy); - return IPSFactoryBuffer_CreateProxy(ps_factory_buffer, outer, riid, ppProxy, ppv); + proxy = heap_alloc(sizeof(*proxy)); + proxy->IRpcProxyBuffer_iface.lpVtbl = &RpcProxyBufferVtbl; + proxy->ref = 1; + + hr = IPSFactoryBuffer_CreateProxy(ps_factory_buffer, outer, riid, &proxy->buffer, ppv); + ok(hr == S_OK, "CreateProxy failed: %08lx\n", hr); + + *ppProxy = &proxy->IRpcProxyBuffer_iface; + + return S_OK; } static HRESULT WINAPI PSFactoryBuffer_CreateStub(IPSFactoryBuffer *iface, REFIID riid, @@ -1327,10 +1414,12 @@ static void test_marshal_channel_buffer(void) SET_EXPECT(CreateStub); SET_EXPECT(CreateProxy); + SET_EXPECT(Connect); hr = IUnknown_QueryInterface(proxy, &IID_IOleWindow, (void**)&ole_window); ok(hr == S_OK, "Could not get IOleWindow iface: %08lx\n", hr); CHECK_CALLED(CreateStub); CHECK_CALLED(CreateProxy); + CHECK_CALLED(Connect); SET_EXPECT(Invoke); SET_EXPECT(GetWindow); @@ -1342,10 +1431,13 @@ static void test_marshal_channel_buffer(void) IOleWindow_Release(ole_window); - SET_EXPECT(Disconnect); + SET_EXPECT(RpcStubBuffer_Disconnect); + SET_EXPECT(RpcProxyBuffer_Disconnect); IUnknown_Release(proxy); todo_wine - CHECK_CALLED(Disconnect); + CHECK_CALLED(RpcStubBuffer_Disconnect); + todo_wine + CHECK_CALLED(RpcProxyBuffer_Disconnect); hr = CoRevokeClassObject(registration_key); ok(hr == S_OK, "CoRevokeClassObject failed: %08lx\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3198
From: Connor McAdams <cmcadams(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54609 Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/combase/combase_private.h | 1 + dlls/combase/marshal.c | 8 +++++--- dlls/combase/stubmanager.c | 25 +++++++++++++++++++++++++ dlls/ole32/tests/marshal.c | 2 +- 4 files changed, 32 insertions(+), 4 deletions(-) diff --git a/dlls/combase/combase_private.h b/dlls/combase/combase_private.h index 52facd44792..d7cba16f3b0 100644 --- a/dlls/combase/combase_private.h +++ b/dlls/combase/combase_private.h @@ -256,4 +256,5 @@ struct ifstub * stub_manager_new_ifstub(struct stub_manager *m, IRpcStubBuffer * HRESULT ipid_get_dispatch_params(const IPID *ipid, struct apartment **stub_apt, struct stub_manager **manager, IRpcStubBuffer **stub, IRpcChannelBuffer **chan, IID *iid, IUnknown **iface) DECLSPEC_HIDDEN; +HRESULT ipid_get_dest_context(const IPID *ipid, MSHCTX *dest_context, void **dest_context_data) DECLSPEC_HIDDEN; HRESULT start_apartment_remote_unknown(struct apartment *apt) DECLSPEC_HIDDEN; diff --git a/dlls/combase/marshal.c b/dlls/combase/marshal.c index e2f6a57d8e1..84f57b8c1c8 100644 --- a/dlls/combase/marshal.c +++ b/dlls/combase/marshal.c @@ -698,7 +698,7 @@ HRESULT WINAPI CoReleaseMarshalData(IStream *stream) } static HRESULT std_unmarshal_interface(MSHCTX dest_context, void *dest_context_data, - IStream *stream, REFIID riid, void **ppv) + IStream *stream, REFIID riid, void **ppv, BOOL dest_context_known) { struct stub_manager *stubmgr = NULL; struct OR_STANDARD obj; @@ -757,6 +757,8 @@ static HRESULT std_unmarshal_interface(MSHCTX dest_context, void *dest_context_d { if (!stub_manager_notify_unmarshal(stubmgr, &obj.std.ipid)) hres = CO_E_OBJNOTCONNECTED; + if (SUCCEEDED(hres) && !dest_context_known) + hres = ipid_get_dest_context(&obj.std.ipid, &dest_context, &dest_context_data); } else { @@ -803,7 +805,7 @@ HRESULT WINAPI CoUnmarshalInterface(IStream *stream, REFIID riid, void **ppv) hr = get_unmarshaler_from_stream(stream, &marshal, &iid); if (hr == S_FALSE) { - hr = std_unmarshal_interface(0, NULL, stream, &iid, (void **)&object); + hr = std_unmarshal_interface(0, NULL, stream, &iid, (void **)&object, FALSE); if (hr != S_OK) ERR("StdMarshal UnmarshalInterface failed, hr %#lx\n", hr); } @@ -2183,7 +2185,7 @@ static HRESULT WINAPI StdMarshalImpl_UnmarshalInterface(IMarshal *iface, IStream return E_NOTIMPL; } - return std_unmarshal_interface(marshal->dest_context, marshal->dest_context_data, stream, riid, ppv); + return std_unmarshal_interface(marshal->dest_context, marshal->dest_context_data, stream, riid, ppv, TRUE); } static HRESULT WINAPI StdMarshalImpl_ReleaseMarshalData(IMarshal *iface, IStream *stream) diff --git a/dlls/combase/stubmanager.c b/dlls/combase/stubmanager.c index ff8937d7bd7..1b079f22fef 100644 --- a/dlls/combase/stubmanager.c +++ b/dlls/combase/stubmanager.c @@ -554,6 +554,31 @@ HRESULT ipid_get_dispatch_params(const IPID *ipid, struct apartment **stub_apt, return S_OK; } +HRESULT ipid_get_dest_context(const IPID *ipid, MSHCTX *dest_context, void **dest_context_data) +{ + struct stub_manager *stubmgr; + struct ifstub *ifstub; + struct apartment *apt; + void *data; + HRESULT hr; + DWORD ctx; + + hr = ipid_to_ifstub(ipid, &apt, &stubmgr, &ifstub); + if (hr != S_OK) return RPC_E_DISCONNECTED; + + hr = IRpcChannelBuffer_GetDestCtx(ifstub->chan, &ctx, &data); + if (SUCCEEDED(hr)) + { + *dest_context = ctx; + *dest_context_data = data; + } + + stub_manager_int_release(stubmgr); + apartment_release(apt); + + return hr; +} + /* returns TRUE if it is possible to unmarshal, FALSE otherwise. */ BOOL stub_manager_notify_unmarshal(struct stub_manager *m, const IPID *ipid) { diff --git a/dlls/ole32/tests/marshal.c b/dlls/ole32/tests/marshal.c index 0ea6148745f..a1e8cf30c83 100644 --- a/dlls/ole32/tests/marshal.c +++ b/dlls/ole32/tests/marshal.c @@ -589,7 +589,7 @@ static HRESULT WINAPI RpcProxyBuffer_Connect(IRpcProxyBuffer *iface, IRpcChannel hr = IRpcChannelBuffer_GetDestCtx(pRpcChannelBuffer, &dest_context, &dest_context_data); ok(hr == S_OK, "GetDestCtx failed: %08lx\n", hr); - todo_wine ok(dest_context == MSHCTX_INPROC, "desc_context = %lx\n", dest_context); + ok(dest_context == MSHCTX_INPROC, "desc_context = %lx\n", dest_context); ok(!dest_context_data, "desc_context_data = %p\n", dest_context_data); return IRpcProxyBuffer_Connect(This->buffer, pRpcChannelBuffer); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3198
On Thu Jun 29 17:15:02 2023 +0000, Alex Henrie wrote:
I just tried it and this fixes <https://bugs.winehq.org/show_bug.cgi?id=54609>. Thank you so much Connor! Could you please add a Wine-Bug line to the second commit? Looking at the log on that bug report it makes sense that this would fix it, since it's also using the free threaded marshaler. Went ahead and added that to the commit. Thanks for testing!
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3198#note_37356
participants (2)
-
Connor McAdams -
Connor McAdams (＠cmcadams)