[PATCH 1/3] uiautomationcore: Implement UiaGetReservedNotSupportedValue.
Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/uiautomationcore/Makefile.in | 1 + dlls/uiautomationcore/uia_main.c | 196 +++++++++++++++++++++++++++++- 2 files changed, 195 insertions(+), 2 deletions(-) diff --git a/dlls/uiautomationcore/Makefile.in b/dlls/uiautomationcore/Makefile.in index 71ea7b99c94..5a72ea144c4 100644 --- a/dlls/uiautomationcore/Makefile.in +++ b/dlls/uiautomationcore/Makefile.in @@ -1,5 +1,6 @@ MODULE = uiautomationcore.dll IMPORTLIB = uiautomationcore +IMPORTS = uuid ole32 EXTRADLLFLAGS = -Wb,--prefer-native diff --git a/dlls/uiautomationcore/uia_main.c b/dlls/uiautomationcore/uia_main.c index 2dada95af80..e51a4b4d555 100644 --- a/dlls/uiautomationcore/uia_main.c +++ b/dlls/uiautomationcore/uia_main.c @@ -16,12 +16,199 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA */ +#define COBJMACROS + #include "uiautomation.h" #include "wine/debug.h" +#include "wine/heap.h" WINE_DEFAULT_DEBUG_CHANNEL(uiautomation); +struct uia_rsrv_ftmarshaler +{ + IMarshal IMarshal_iface; + + IUnknown *inner_unk; + IMarshal *marshal; +}; + +/* + * Create a custom proxy IMarshal, so that IMarshal::Release actually releases + * the interface. + */ +static struct uia_rsrv_ftmarshaler *impl_uia_rsrv_ft_from_IMarshal(IMarshal *iface) +{ + return CONTAINING_RECORD(iface, struct uia_rsrv_ftmarshaler, IMarshal_iface); +} + +static HRESULT WINAPI uia_rsrv_ftmarshaler_QueryInterface(IMarshal *iface, REFIID riid, void **obj) +{ + struct uia_rsrv_ftmarshaler *marshaler = impl_uia_rsrv_ft_from_IMarshal(iface); + + TRACE("%p, %s, %p\n", iface, debugstr_guid(riid), obj); + + return IMarshal_QueryInterface(marshaler->marshal, riid, obj); +} + +static ULONG WINAPI uia_rsrv_ftmarshaler_AddRef(IMarshal *iface) +{ + struct uia_rsrv_ftmarshaler *marshaler = impl_uia_rsrv_ft_from_IMarshal(iface); + ULONG ref = IUnknown_AddRef(marshaler->inner_unk); + + TRACE("%p, ref %d\n", iface, ref); + + return ref; +} + +static ULONG WINAPI uia_rsrv_ftmarshaler_Release(IMarshal *iface) +{ + struct uia_rsrv_ftmarshaler *marshaler = impl_uia_rsrv_ft_from_IMarshal(iface); + ULONG ref = IUnknown_Release(marshaler->inner_unk); + + TRACE("%p, ref %d\n", iface, ref); + + if (!ref) + heap_free(marshaler); + + return ref; +} + +static HRESULT WINAPI uia_rsrv_ftmarshaler_GetUnmarshalClass(IMarshal *iface, REFIID riid, void *pv, + DWORD dest_context, void *pvDestContext, DWORD mshlflags, CLSID *clsid) +{ + struct uia_rsrv_ftmarshaler *marshaler = impl_uia_rsrv_ft_from_IMarshal(iface); + TRACE("%s, %p, %#x, %p, %#x, %p\n", debugstr_guid(riid), pv, dest_context, pvDestContext, mshlflags, clsid); + return IMarshal_GetUnmarshalClass(marshaler->marshal, riid, pv, dest_context, pvDestContext, mshlflags, clsid); +} + +static HRESULT WINAPI uia_rsrv_ftmarshaler_GetMarshalSizeMax(IMarshal *iface, REFIID riid, void *pv, + DWORD dest_context, void *pvDestContext, DWORD mshlflags, DWORD *size) +{ + struct uia_rsrv_ftmarshaler *marshaler = impl_uia_rsrv_ft_from_IMarshal(iface); + TRACE("%s, %p, %#x, %p, %#x, %p\n", debugstr_guid(riid), pv, dest_context, pvDestContext, mshlflags, size); + return IMarshal_GetMarshalSizeMax(marshaler->marshal, riid, pv, dest_context, pvDestContext, mshlflags, size); +} + +static HRESULT WINAPI uia_rsrv_ftmarshaler_MarshalInterface(IMarshal *iface, IStream *stream, REFIID riid, + void *pv, DWORD dest_context, void *pvDestContext, DWORD mshlflags) +{ + struct uia_rsrv_ftmarshaler *marshaler = impl_uia_rsrv_ft_from_IMarshal(iface); + TRACE("%p, %s, %p, %#x, %p, %#x\n", stream, debugstr_guid(riid), pv, dest_context, pvDestContext, mshlflags); + return IMarshal_MarshalInterface(marshaler->marshal, stream, riid, pv, dest_context, pvDestContext, mshlflags); +} + +static HRESULT WINAPI uia_rsrv_ftmarshaler_UnmarshalInterface(IMarshal *iface, IStream *stream, REFIID riid, void **ppv) +{ + struct uia_rsrv_ftmarshaler *marshaler = impl_uia_rsrv_ft_from_IMarshal(iface); + TRACE("%p, %s, %p\n", stream, debugstr_guid(riid), ppv); + return IMarshal_UnmarshalInterface(marshaler->marshal, stream, riid, ppv); +} + +static HRESULT WINAPI uia_rsrv_ftmarshaler_ReleaseMarshalData(IMarshal *iface, IStream *stream) +{ + struct uia_rsrv_ftmarshaler *marshaler = impl_uia_rsrv_ft_from_IMarshal(iface); + TRACE("%p\n", stream); + return IMarshal_ReleaseMarshalData(marshaler->marshal, stream); +} + +static HRESULT WINAPI uia_rsrv_ftmarshaler_DisconnectObject(IMarshal *iface, DWORD reserved) +{ + struct uia_rsrv_ftmarshaler *marshaler = impl_uia_rsrv_ft_from_IMarshal(iface); + TRACE("\n"); + return IMarshal_DisconnectObject(marshaler->marshal, reserved); +} + +static const IMarshalVtbl uia_rsrv_ftmarshaler_vtbl = +{ + uia_rsrv_ftmarshaler_QueryInterface, + uia_rsrv_ftmarshaler_AddRef, + uia_rsrv_ftmarshaler_Release, + uia_rsrv_ftmarshaler_GetUnmarshalClass, + uia_rsrv_ftmarshaler_GetMarshalSizeMax, + uia_rsrv_ftmarshaler_MarshalInterface, + uia_rsrv_ftmarshaler_UnmarshalInterface, + uia_rsrv_ftmarshaler_ReleaseMarshalData, + uia_rsrv_ftmarshaler_DisconnectObject +}; + +/* + * When passing the ReservedNotSupportedValue/ReservedMixedAttributeValue + * interface pointers across apartments within the same process, use the + * free threaded marshaler to preserve the pointer value. Wrap the IMarshal + * interface received by CoCreateFreeThreadedMarshaler so that we can maintain + * our own reference count and release the object when it hits 0. + */ +static HRESULT uia_reserved_val_create_ftm(IUnknown *outer, IMarshal **marshaler) +{ + struct uia_rsrv_ftmarshaler *object; + IUnknown *ftm_unk; + HRESULT hr; + + TRACE("%p, %p\n", outer, marshaler); + + hr = CoCreateFreeThreadedMarshaler(outer, &ftm_unk); + if (FAILED(hr)) return hr; + + object = heap_alloc(sizeof(*object)); + if (!object) + { + IUnknown_Release(ftm_unk); + return E_OUTOFMEMORY; + } + + object->IMarshal_iface.lpVtbl = &uia_rsrv_ftmarshaler_vtbl; + object->inner_unk = ftm_unk; + hr = IUnknown_QueryInterface(ftm_unk, &IID_IMarshal, (void **)&object->marshal); + if (FAILED(hr)) + FIXME("Failed to QI for IMarshal.\n"); + + *marshaler = &object->IMarshal_iface; + + return S_OK; +} + +/* + * UiaReservedNotSupported object. + */ +static HRESULT WINAPI uia_reserved_ns_QueryInterface(IUnknown *iface, + REFIID riid, void **ppv) +{ + *ppv = NULL; + if (IsEqualIID(riid, &IID_IUnknown)) + *ppv = iface; + else if (IsEqualIID(riid, &IID_IMarshal)) + { + IMarshal *ftm; + + if (FAILED(uia_reserved_val_create_ftm(iface, &ftm))) + return E_NOINTERFACE; + *ppv = ftm; + } + else + return E_NOINTERFACE; + + return S_OK; +} + +static ULONG WINAPI uia_reserved_ns_AddRef(IUnknown *iface) +{ + return 1; +} + +static ULONG WINAPI uia_reserved_ns_Release(IUnknown *iface) +{ + return 1; +} + +static const IUnknownVtbl uia_reserved_ns_vtbl = { + uia_reserved_ns_QueryInterface, + uia_reserved_ns_AddRef, + uia_reserved_ns_Release, +}; + +static IUnknown uia_reserved_ns_iface = {&uia_reserved_ns_vtbl}; + /*********************************************************************** * UiaClientsAreListening (uiautomationcore.@) */ @@ -46,8 +233,13 @@ HRESULT WINAPI UiaGetReservedMixedAttributeValue(IUnknown **value) */ HRESULT WINAPI UiaGetReservedNotSupportedValue(IUnknown **value) { - FIXME("(%p) stub!\n", value); - *value = NULL; + TRACE("(%p)\n", value); + + if (!value) + return E_INVALIDARG; + + *value = &uia_reserved_ns_iface; + return S_OK; } -- 2.25.1
Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/uiautomationcore/uia_main.c | 50 ++++++++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 2 deletions(-) diff --git a/dlls/uiautomationcore/uia_main.c b/dlls/uiautomationcore/uia_main.c index e51a4b4d555..0294357c28a 100644 --- a/dlls/uiautomationcore/uia_main.c +++ b/dlls/uiautomationcore/uia_main.c @@ -209,6 +209,47 @@ static const IUnknownVtbl uia_reserved_ns_vtbl = { static IUnknown uia_reserved_ns_iface = {&uia_reserved_ns_vtbl}; +/* + * UiaReservedMixedAttribute object. + */ +static HRESULT WINAPI uia_reserved_ma_QueryInterface(IUnknown *iface, + REFIID riid, void **ppv) +{ + *ppv = NULL; + if (IsEqualIID(riid, &IID_IUnknown)) + *ppv = iface; + else if (IsEqualIID(riid, &IID_IMarshal)) + { + IMarshal *ftm; + + if (FAILED(uia_reserved_val_create_ftm(iface, &ftm))) + return E_NOINTERFACE; + *ppv = ftm; + } + else + return E_NOINTERFACE; + + return S_OK; +} + +static ULONG WINAPI uia_reserved_ma_AddRef(IUnknown *iface) +{ + return 1; +} + +static ULONG WINAPI uia_reserved_ma_Release(IUnknown *iface) +{ + return 1; +} + +static const IUnknownVtbl uia_reserved_ma_vtbl = { + uia_reserved_ma_QueryInterface, + uia_reserved_ma_AddRef, + uia_reserved_ma_Release, +}; + +static IUnknown uia_reserved_ma_iface = {&uia_reserved_ma_vtbl}; + /*********************************************************************** * UiaClientsAreListening (uiautomationcore.@) */ @@ -223,8 +264,13 @@ BOOL WINAPI UiaClientsAreListening(void) */ HRESULT WINAPI UiaGetReservedMixedAttributeValue(IUnknown **value) { - FIXME("(%p) stub!\n", value); - *value = NULL; + TRACE("(%p)\n", value); + + if (!value) + return E_INVALIDARG; + + *value = &uia_reserved_ma_iface; + return S_OK; } -- 2.25.1
Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/uiautomationcore/tests/Makefile.in | 2 +- dlls/uiautomationcore/tests/uiautomation.c | 163 +++++++++++++++++++++ 2 files changed, 164 insertions(+), 1 deletion(-) diff --git a/dlls/uiautomationcore/tests/Makefile.in b/dlls/uiautomationcore/tests/Makefile.in index c39b062b6fd..1d08e19093f 100644 --- a/dlls/uiautomationcore/tests/Makefile.in +++ b/dlls/uiautomationcore/tests/Makefile.in @@ -1,5 +1,5 @@ TESTDLL = uiautomationcore.dll -IMPORTS = uiautomationcore user32 +IMPORTS = uiautomationcore user32 ole32 C_SRCS = \ uiautomation.c diff --git a/dlls/uiautomationcore/tests/uiautomation.c b/dlls/uiautomationcore/tests/uiautomation.c index cbcba1af294..d10e28b7a85 100644 --- a/dlls/uiautomationcore/tests/uiautomation.c +++ b/dlls/uiautomationcore/tests/uiautomation.c @@ -86,7 +86,170 @@ if (hr == S_OK) UnregisterClassA("HostProviderFromHwnd class", NULL); } +static DWORD WINAPI uia_reserved_val_iface_marshal_thread(LPVOID param) +{ + IStream **stream = param; + IUnknown *unk_ns, *unk_ns2, *unk_ma, *unk_ma2; + HRESULT hr; + + CoInitializeEx(NULL, COINIT_MULTITHREADED); + + hr = CoGetInterfaceAndReleaseStream(stream[0], &IID_IUnknown, (void **)&unk_ns); + ok(hr == S_OK, "Unexpected hr %#x.\n", hr); + + hr = CoGetInterfaceAndReleaseStream(stream[1], &IID_IUnknown, (void **)&unk_ma); + ok(hr == S_OK, "Unexpected hr %#x.\n", hr); + + hr = UiaGetReservedNotSupportedValue(&unk_ns2); + ok(hr == S_OK, "Unexpected hr %#x.\n", hr); + + hr = UiaGetReservedMixedAttributeValue(&unk_ma2); + ok(hr == S_OK, "Unexpected hr %#x.\n", hr); + + ok(unk_ns2 == unk_ns, "UiaGetReservedNotSupported pointer mismatch, unk_ns2 %p, unk_ns %p\n", unk_ns2, unk_ns); + ok(unk_ma2 == unk_ma, "UiaGetReservedMixedAttribute pointer mismatch, unk_ma2 %p, unk_ma %p\n", unk_ma2, unk_ma); + + CoUninitialize(); + + return 0; +} + +static void test_uia_reserved_value_ifaces(void) +{ + IUnknown *unk_ns, *unk_ns2, *unk_ma, *unk_ma2; + IMarshal *marshal, *marshal2; + IStream *stream[2]; + HANDLE thread; + ULONG refcnt; + HRESULT hr; + + /* ReservedNotSupportedValue. */ + hr = UiaGetReservedNotSupportedValue(NULL); + ok(hr == E_INVALIDARG, "Unexpected hr %#x.\n", hr); + + hr = UiaGetReservedNotSupportedValue(&unk_ns); + ok(hr == S_OK, "Unexpected hr %#x.\n", hr); + ok(unk_ns != NULL, "UiaGetReservedNotSupportedValue returned NULL interface.\n"); + + refcnt = IUnknown_AddRef(unk_ns); + ok(refcnt == 1, "Expected refcnt %d, got %d\n", 1, refcnt); + + refcnt = IUnknown_AddRef(unk_ns); + ok(refcnt == 1, "Expected refcnt %d, got %d\n", 1, refcnt); + + refcnt = IUnknown_Release(unk_ns); + ok(refcnt == 1, "Expected refcnt %d, got %d\n", 1, refcnt); + + hr = UiaGetReservedNotSupportedValue(&unk_ns2); + ok(hr == S_OK, "Unexpected hr %#x.\n", hr); + ok(unk_ns2 != NULL, "UiaGetReservedNotSupportedValue returned NULL interface."); + ok(unk_ns2 == unk_ns, "UiaGetReservedNotSupported pointer mismatch, unk_ns2 %p, unk_ns %p\n", unk_ns2, unk_ns); + + marshal = marshal2 = NULL; + hr = IUnknown_QueryInterface(unk_ns, &IID_IMarshal, (void **)&marshal); + ok(hr == S_OK, "Unexpected hr %#x.\n", hr); + ok(marshal != NULL, "Failed to get IMarshal interface.\n"); + + hr = IUnknown_QueryInterface(unk_ns, &IID_IMarshal, (void **)&marshal2); + ok(hr == S_OK, "Unexpected hr %#x.\n", hr); + ok(marshal2 != NULL, "Failed to get IMarshal interface.\n"); + + /* Unique IMarshal interface is created each time we QI for IMarshal. */ + ok(marshal != marshal2, "Expected unique pointers for IMarshal interface, got %p and %p.\n", marshal, marshal2); + + refcnt = IMarshal_AddRef(marshal); + ok(refcnt == 2, "Expected refcnt %d, got %d\n", 2, refcnt); + + refcnt = IMarshal_Release(marshal); + ok(refcnt == 1, "Expected refcnt %d, got %d\n", 1, refcnt); + + refcnt = IMarshal_Release(marshal); + ok(refcnt == 0, "Expected refcnt %d, got %d\n", 0, refcnt); + + refcnt = IMarshal_AddRef(marshal2); + ok(refcnt == 2, "Expected refcnt %d, got %d\n", 2, refcnt); + + refcnt = IMarshal_Release(marshal2); + ok(refcnt == 1, "Expected refcnt %d, got %d\n", 1, refcnt); + + refcnt = IMarshal_Release(marshal2); + ok(refcnt == 0, "Expected refcnt %d, got %d\n", 0, refcnt); + + /* ReservedMixedAttributeValue. */ + hr = UiaGetReservedMixedAttributeValue(NULL); + ok(hr == E_INVALIDARG, "Unexpected hr %#x.\n", hr); + + hr = UiaGetReservedMixedAttributeValue(&unk_ma); + ok(hr == S_OK, "Unexpected hr %#x.\n", hr); + ok(unk_ma != NULL, "UiaGetReservedMixedAttributeValue returned NULL interface."); + + refcnt = IUnknown_AddRef(unk_ma); + ok(refcnt == 1, "Expected refcnt %d, got %d\n", 1, refcnt); + + refcnt = IUnknown_AddRef(unk_ma); + ok(refcnt == 1, "Expected refcnt %d, got %d\n", 1, refcnt); + + refcnt = IUnknown_Release(unk_ma); + ok(refcnt == 1, "Expected refcnt %d, got %d\n", 1, refcnt); + + hr = UiaGetReservedMixedAttributeValue(&unk_ma2); + ok(hr == S_OK, "Unexpected hr %#x.\n", hr); + ok(unk_ma2 != NULL, "UiaGetReservedMixedAttributeValue returned NULL interface."); + ok(unk_ma2 == unk_ma, "UiaGetReservedMixedAttribute pointer mismatch, unk_ma2 %p, unk_ma %p\n", unk_ma2, unk_ma); + + marshal = marshal2 = NULL; + hr = IUnknown_QueryInterface(unk_ma, &IID_IMarshal, (void **)&marshal); + ok(hr == S_OK, "Unexpected hr %#x.\n", hr); + ok(marshal != NULL, "Failed to get IMarshal interface.\n"); + + hr = IUnknown_QueryInterface(unk_ma, &IID_IMarshal, (void **)&marshal2); + ok(hr == S_OK, "Unexpected hr %#x.\n", hr); + ok(marshal2 != NULL, "Failed to get IMarshal interface.\n"); + ok(marshal != marshal2, "Expected unique pointers for IMarshal interface, got %p and %p.\n", marshal, marshal2); + + refcnt = IMarshal_AddRef(marshal); + ok(refcnt == 2, "Expected refcnt %d, got %d\n", 2, refcnt); + + refcnt = IMarshal_Release(marshal); + ok(refcnt == 1, "Expected refcnt %d, got %d\n", 1, refcnt); + + refcnt = IMarshal_Release(marshal); + ok(refcnt == 0, "Expected refcnt %d, got %d\n", 0, refcnt); + + refcnt = IMarshal_AddRef(marshal2); + ok(refcnt == 2, "Expected refcnt %d, got %d\n", 2, refcnt); + + refcnt = IMarshal_Release(marshal2); + ok(refcnt == 1, "Expected refcnt %d, got %d\n", 1, refcnt); + + refcnt = IMarshal_Release(marshal2); + ok(refcnt == 0, "Expected refcnt %d, got %d\n", 0, refcnt); + + /* Test cross-thread marshaling behavior. */ + CoInitializeEx(NULL, COINIT_APARTMENTTHREADED); + + hr = CoMarshalInterThreadInterfaceInStream(&IID_IUnknown, unk_ns, &stream[0]); + ok(hr == S_OK, "Unexpected hr %#x.\n", hr); + hr = CoMarshalInterThreadInterfaceInStream(&IID_IUnknown, unk_ma, &stream[1]); + ok(hr == S_OK, "Unexpected hr %#x.\n", hr); + + thread = CreateThread(NULL, 0, uia_reserved_val_iface_marshal_thread, (void *)stream, 0, NULL); + while (MsgWaitForMultipleObjects(1, &thread, FALSE, INFINITE, QS_ALLINPUT) != WAIT_OBJECT_0) + { + MSG msg; + while(PeekMessageW(&msg, 0, 0, 0, PM_REMOVE)) + { + TranslateMessage(&msg); + DispatchMessageW(&msg); + } + } + CloseHandle(thread); + + CoUninitialize(); +} + START_TEST(uiautomation) { test_UiaHostProviderFromHwnd(); + test_uia_reserved_value_ifaces(); } -- 2.25.1
On 10/27/21 10:55 PM, Connor McAdams wrote:
+/* + * When passing the ReservedNotSupportedValue/ReservedMixedAttributeValue + * interface pointers across apartments within the same process, use the + * free threaded marshaler to preserve the pointer value. Wrap the IMarshal + * interface received by CoCreateFreeThreadedMarshaler so that we can maintain + * our own reference count and release the object when it hits 0. + */ +static HRESULT uia_reserved_val_create_ftm(IUnknown *outer, IMarshal **marshaler) I don't understand this part, if every QI for IMarshal returns new instance, why do need a wrapper? And which object do you mean to release, with our own reference count?
On Wed, Oct 27, 2021 at 11:12:31PM +0300, Nikolay Sivov wrote:
On 10/27/21 10:55 PM, Connor McAdams wrote:
+/* + * When passing the ReservedNotSupportedValue/ReservedMixedAttributeValue + * interface pointers across apartments within the same process, use the + * free threaded marshaler to preserve the pointer value. Wrap the IMarshal + * interface received by CoCreateFreeThreadedMarshaler so that we can maintain + * our own reference count and release the object when it hits 0. + */ +static HRESULT uia_reserved_val_create_ftm(IUnknown *outer, IMarshal **marshaler) I don't understand this part, if every QI for IMarshal returns new instance, why do need a wrapper? And which object do you mean to release, with our own reference count?
CoCreateFreeThreadedMarshaler with a non-NULL outer argument returns an IMarshal interface that has its reference count tied to the reference count of the outer object. In this case, AddRef/Release on the reserved object is pointless, because it is a static object whose reference count doesn't (and probably shouldn't) change. Normally, when the outer objects reference count hits 0, you'd release the unique inner IUnknown interface created by CoCreateFreeThreadedMarshaler there. We can't do that here, so my only idea was to create a unique IMarshal 'wrapper' interface with its own reference count, so that when it hits 0, we release the inner IUnknown object and free the memory.
On 10/27/21 11:24 PM, Connor McAdams wrote:
On Wed, Oct 27, 2021 at 11:12:31PM +0300, Nikolay Sivov wrote:
On 10/27/21 10:55 PM, Connor McAdams wrote:
+/* + * When passing the ReservedNotSupportedValue/ReservedMixedAttributeValue + * interface pointers across apartments within the same process, use the + * free threaded marshaler to preserve the pointer value. Wrap the IMarshal + * interface received by CoCreateFreeThreadedMarshaler so that we can maintain + * our own reference count and release the object when it hits 0. + */ +static HRESULT uia_reserved_val_create_ftm(IUnknown *outer, IMarshal **marshaler) I don't understand this part, if every QI for IMarshal returns new instance, why do need a wrapper? And which object do you mean to release, with our own reference count?
CoCreateFreeThreadedMarshaler with a non-NULL outer argument returns an IMarshal interface that has its reference count tied to the reference count of the outer object. In this case, AddRef/Release on the reserved object is pointless, because it is a static object whose reference count doesn't (and probably shouldn't) change.
Normally, when the outer objects reference count hits 0, you'd release the unique inner IUnknown interface created by CoCreateFreeThreadedMarshaler there. We can't do that here, so my only idea was to create a unique IMarshal 'wrapper' interface with its own reference count, so that when it hits 0, we release the inner IUnknown object and free the memory. I see. Is that really important that QI returns new instance every time? It seems fine to ignore that detail, and if you do, you can maintain one marshaler instance per static object type, releasing it on 0 refcount. Could be some cleanup function instead to release then all at once, if it's just one per "reserved" object, and not bother with artificial refcounting at all.
On Wed, Oct 27, 2021 at 11:49:51PM +0300, Nikolay Sivov wrote:
On 10/27/21 11:24 PM, Connor McAdams wrote:
On Wed, Oct 27, 2021 at 11:12:31PM +0300, Nikolay Sivov wrote:
On 10/27/21 10:55 PM, Connor McAdams wrote:
+/* + * When passing the ReservedNotSupportedValue/ReservedMixedAttributeValue + * interface pointers across apartments within the same process, use the + * free threaded marshaler to preserve the pointer value. Wrap the IMarshal + * interface received by CoCreateFreeThreadedMarshaler so that we can maintain + * our own reference count and release the object when it hits 0. + */ +static HRESULT uia_reserved_val_create_ftm(IUnknown *outer, IMarshal **marshaler) I don't understand this part, if every QI for IMarshal returns new instance, why do need a wrapper? And which object do you mean to release, with our own reference count?
CoCreateFreeThreadedMarshaler with a non-NULL outer argument returns an IMarshal interface that has its reference count tied to the reference count of the outer object. In this case, AddRef/Release on the reserved object is pointless, because it is a static object whose reference count doesn't (and probably shouldn't) change.
Normally, when the outer objects reference count hits 0, you'd release the unique inner IUnknown interface created by CoCreateFreeThreadedMarshaler there. We can't do that here, so my only idea was to create a unique IMarshal 'wrapper' interface with its own reference count, so that when it hits 0, we release the inner IUnknown object and free the memory. I see. Is that really important that QI returns new instance every time? It seems fine to ignore that detail, and if you do, you can maintain one marshaler instance per static object type, releasing it on 0 refcount. Could be some cleanup function instead to release then all at once, if it's just one per "reserved" object, and not bother with artificial refcounting at all.
Do you mean something like, when the reserved object is QI'd for an IMarshal interface, we check if we've already created one, and if not, we create one? And then subsequent QI's for IMarshal will return the same interface? I guess that wouldn't be a problem, although it might cause weirdness if someone called Release too many times on the shared IMarshal and caused it to get freed while another thread/instance is using it. I can't think of a circumstance where this would happen, but that would fail with the approach I believe you're suggesting.
On 10/28/21 12:01 AM, Connor McAdams wrote:
On Wed, Oct 27, 2021 at 11:49:51PM +0300, Nikolay Sivov wrote:
On 10/27/21 11:24 PM, Connor McAdams wrote:
On Wed, Oct 27, 2021 at 11:12:31PM +0300, Nikolay Sivov wrote:
On 10/27/21 10:55 PM, Connor McAdams wrote:
+/* + * When passing the ReservedNotSupportedValue/ReservedMixedAttributeValue + * interface pointers across apartments within the same process, use the + * free threaded marshaler to preserve the pointer value. Wrap the IMarshal + * interface received by CoCreateFreeThreadedMarshaler so that we can maintain + * our own reference count and release the object when it hits 0. + */ +static HRESULT uia_reserved_val_create_ftm(IUnknown *outer, IMarshal **marshaler) I don't understand this part, if every QI for IMarshal returns new instance, why do need a wrapper? And which object do you mean to release, with our own reference count?
CoCreateFreeThreadedMarshaler with a non-NULL outer argument returns an IMarshal interface that has its reference count tied to the reference count of the outer object. In this case, AddRef/Release on the reserved object is pointless, because it is a static object whose reference count doesn't (and probably shouldn't) change.
Normally, when the outer objects reference count hits 0, you'd release the unique inner IUnknown interface created by CoCreateFreeThreadedMarshaler there. We can't do that here, so my only idea was to create a unique IMarshal 'wrapper' interface with its own reference count, so that when it hits 0, we release the inner IUnknown object and free the memory. I see. Is that really important that QI returns new instance every time? It seems fine to ignore that detail, and if you do, you can maintain one marshaler instance per static object type, releasing it on 0 refcount. Could be some cleanup function instead to release then all at once, if it's just one per "reserved" object, and not bother with artificial refcounting at all.
Do you mean something like, when the reserved object is QI'd for an IMarshal interface, we check if we've already created one, and if not, we create one? And then subsequent QI's for IMarshal will return the same interface?
I guess that wouldn't be a problem, although it might cause weirdness if someone called Release too many times on the shared IMarshal and caused it to get freed while another thread/instance is using it. I can't think of a circumstance where this would happen, but that would fail with the approach I believe you're suggesting. Yes, that makes sense. Is it really necessary to wrap whole IMarshal though?
If you only need lifetime behaviour, maybe something like this is enough: struct reserved_outer { IUnknown IUnknown_iface; LONG refcount; IUnknown *marshaler; <- what CoCreateFreeThreadedMarshaler returns IUnknown *object; <- result of UiaGetReservedNotSupportedValue } outer_QI() -> object->QI() outer->addref -> ++ outer->release -> --, IUnknown_Release(marshaler), free() reserved_QI(IID_IMarshal) -> create new reserved_outer -> create new marshaler instance You
participants (2)
-
Connor McAdams -
Nikolay Sivov