[PATCH v2] ole32: Store proxy/stub CLSIDs per process, not per apartment.
Zebediah Figura
z.figura12 at gmail.com
Wed Aug 2 10:26:37 CDT 2017
On 08/02/2017 04:32 AM, Huw Davies wrote:
> On Tue, Aug 01, 2017 at 04:54:45PM -0500, Zebediah Figura wrote:
>> v2: use proper synchronization; add an MTA test (thanks Sebastian)
>> Signed-off-by: Zebediah Figura <zfigura at codeweavers.com>
>> ---
>> dlls/ole32/compobj.c | 48 ++++++++++++++++++++++++++++++------------
>> dlls/ole32/compobj_private.h | 1 -
>> dlls/ole32/tests/compobj.c | 50 ++++++++++++++++++++++++++++++++++++++++++--
>> 3 files changed, 83 insertions(+), 16 deletions(-)
>>
>> diff --git a/dlls/ole32/compobj.c b/dlls/ole32/compobj.c
>> index 60244485249..ee904adbc38 100644
>> --- a/dlls/ole32/compobj.c
>> +++ b/dlls/ole32/compobj.c
>> @@ -170,8 +170,20 @@ struct registered_psclsid
>> struct list entry;
>> IID iid;
>> CLSID clsid;
>> + OXID apartment_id;
>> };
>>
>> +static struct list RegisteredPSCLSIDList = LIST_INIT(RegisteredPSCLSIDList);
>
> Please don't use CamelCase.
>
>> +
>> +static CRITICAL_SECTION csRegisteredPSCLSIDList;
>> +static CRITICAL_SECTION_DEBUG psclsid_cs_debug =
>> +{
>> + 0, 0, &csRegisteredPSCLSIDList,
>> + { &psclsid_cs_debug.ProcessLocksList, &psclsid_cs_debug.ProcessLocksList },
>> + 0, 0, { (DWORD_PTR)(__FILE__ ": csRegisteredPSCLSIDList") }
>> +};
>> +static CRITICAL_SECTION csRegisteredPSCLSIDList = { &psclsid_cs_debug, -1, 0, 0, 0, 0 };
>> +
>> /*
>> * This is a marshallable object exposing registered local servers.
>> * IServiceProvider is used only because it happens meet requirements
>> @@ -622,7 +634,6 @@ static APARTMENT *apartment_construct(DWORD model)
>>
>> list_init(&apt->proxies);
>> list_init(&apt->stubmgrs);
>> - list_init(&apt->psclsids);
>> list_init(&apt->loaded_dlls);
>> apt->ipidc = 0;
>> apt->refs = 1;
>> @@ -1173,15 +1184,22 @@ DWORD apartment_release(struct apartment *apt)
>> stub_manager_int_release(stubmgr);
>> }
>>
>> - LIST_FOR_EACH_SAFE(cursor, cursor2, &apt->psclsids)
>> + EnterCriticalSection(&csRegisteredPSCLSIDList);
>> +
>> + LIST_FOR_EACH_SAFE(cursor, cursor2, &RegisteredPSCLSIDList)
>> {
>> struct registered_psclsid *registered_psclsid =
>> LIST_ENTRY(cursor, struct registered_psclsid, entry);
>>
>> - list_remove(®istered_psclsid->entry);
>> - HeapFree(GetProcessHeap(), 0, registered_psclsid);
>> + if (registered_psclsid->apartment_id == apt->oxid)
>> + {
>> + list_remove(®istered_psclsid->entry);
>> + HeapFree(GetProcessHeap(), 0, registered_psclsid);
>> + }
>> }
>>
>> + LeaveCriticalSection(&csRegisteredPSCLSIDList);
>> +
>> /* if this assert fires, then another thread took a reference to a
>> * stub manager without taking a reference to the containing
>> * apartment, which it must do. */
>
>
> It seems it's more complicated than this. Consider two already
> existing STAs, if the PSClsid is registered in STA1 which then leaves
> the apt, a call in STA2 to CoGetPSCLSID() still succeeds.
>
> You'll want to expand the tests to cover things like this to really figure out
> what's going on.
>
> Also note that the MTA -> MTA test isn't very convincing, since there is only
> one MTA and hence both threads are in the same apt.
>
> Huw.
>
>
I see; I'll send an updated patch.
More information about the wine-devel
mailing list