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(a)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.