[PATCH 1/2] ole32: Implement chained IInitializeSpy support.

[Nikolay Sivov]

On 1/24/19 3:00 PM, Huw Davies wrote:

> On Wed, Jan 23, 2019 at 11:41:44AM +0300, Nikolay Sivov wrote:
>> Signed-off-by: Nikolay Sivov <nsivov at codeweavers.com>
>> ---
>>   dlls/ole32/compobj.c         | 123 +++++++++++++++++++++++++++--------
>>   dlls/ole32/compobj_private.h |  27 +++++++-
>>   dlls/ole32/tests/compobj.c   |  15 ++---
>>   3 files changed, 126 insertions(+), 39 deletions(-)
>>
>> diff --git a/dlls/ole32/compobj.c b/dlls/ole32/compobj.c
>> index 66b1683d98..c65250442d 100644
>> --- a/dlls/ole32/compobj.c
>> +++ b/dlls/ole32/compobj.c
>> @@ -1827,14 +1856,42 @@ HRESULT WINAPI CoRegisterInitializeSpy(IInitializeSpy *spy, ULARGE_INTEGER *cook
>>   HRESULT WINAPI CoRevokeInitializeSpy(ULARGE_INTEGER cookie)
>>   {
>>       struct oletls *info = COM_CurrentInfo();
>> +    struct init_spy *spy;
>> +
>>       TRACE("(%s)\n", wine_dbgstr_longlong(cookie.QuadPart));
>>   
>> -    if (!info || !info->spy || cookie.QuadPart != (DWORD_PTR)info->spy)
>> +    if (!info || list_empty(&info->spies.active))
>>           return E_INVALIDARG;
>>   
>> -    IInitializeSpy_Release(info->spy);
>> -    info->spy = NULL;
>> -    return S_OK;
>> +    if (cookie.HighPart != GetCurrentThreadId() || cookie.LowPart >= info->spies.next_id)
>> +        return E_INVALIDARG;
>> +
>> +    LIST_FOR_EACH_ENTRY(spy, &info->spies.active, struct init_spy, entry)
>> +    {
>> +        if (cookie.LowPart == spy->id)
>> +        {
>> +            IInitializeSpy_Release(spy->spy);
>> +            spy->spy = NULL;
>> +            list_remove(&spy->entry);
>> +
>> +            /* Move to freed list, unless it was latest item id or last item in the active list. */
>> +            if (spy->id == info->spies.next_id - 1)
>> +                info->spies.next_id--;
>> +            else if (list_empty(&info->spies.active))
>> +                oletls_release_spy_list(&info->spies.freed);
>> +            else
>> +            {
>> +                list_add_tail(&info->spies.freed, &spy->entry);
>> +                spy = NULL;
>> +            }
>> +
>> +            heap_free(spy);
>> +
>> +            return S_OK;
>> +        }
>> +    }
>> +
>> +    return E_INVALIDARG;
>>   }
> I don't think the extra complication of a free list is worth it.  How
> about just keeping the active list sorted, when you register a new
> spy just iterate through until you find a missing id and insert the
> new node at that point?  This stuff is not going to be performance
> critical.

Sure, will do.

>
>
>> diff --git a/dlls/ole32/compobj_private.h b/dlls/ole32/compobj_private.h
>> index 1e564a3de1..1829866bff 100644
>> --- a/dlls/ole32/compobj_private.h
>> +++ b/dlls/ole32/compobj_private.h
>>   struct oletls
>>   {
>> @@ -160,7 +174,7 @@ struct oletls
>>       IErrorInfo       *errorinfo;   /* see errorinfo.c */
>>       IUnknown         *state;       /* see CoSetState */
>>       DWORD             apt_mask;    /* apartment mask (+0Ch on x86) */
>> -    IInitializeSpy   *spy;         /* The "SPY" from CoInitializeSpy */
>> +    void            *unknown0;     /* The "SPY" from CoInitializeSpy */
> Could you remove the comment?
I will.
>
>>       DWORD            inits;        /* number of times CoInitializeEx called */
>>       DWORD            ole_inits;    /* number of times OleInitialize called */
>>       GUID             causality_id; /* unique identifier for each COM call */
>> @@ -171,6 +185,7 @@ struct oletls
>>       IUnknown        *call_state;    /* current call context (+3Ch on x86) */
>>       DWORD            unknown2[46];
>>       IUnknown        *cancel_object; /* cancel object set by CoSetCancelObject (+F8h on x86) */
>> +    struct spy_chain spies;         /* Spies from CoRegisterInitializeSpy */
>>   };
> FWIW, it's not clear whether anything needs compatiblity with native's
> layout, some things appear to have changed anyway.  For example,
> 'inits' is at 0x18 in 32-bit Windows 7; we have it at 0x14.

Probably. One use case that I'm aware of is dotnet/coreclr, they are 
using at least one field directly there, that's why I didn't want to 
touch existing layout [1].

But admittedly I don't know if this code path is active in practice, or 
if this code is used in .NET Core/Mono at all.

[1] SetupOleContext() in https://github.com/dotnet/coreclr

>
> Huw.