[PATCH 1/3] uiautomationcore: Implement UiaGetReservedNotSupportedValue.

[Nikolay Sivov]

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