New subject: [PATCH v2] ole32: Store proxy/stub CLSIDs per process, not per apartment.

2 Aug 2017

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@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(&registered_psclsid->entry);


       HeapFree(GetProcessHeap(), 0, registered_psclsid);




       if (registered_psclsid->apartment_id == apt->oxid)


       {


           list_remove(&registered_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.

    

Re: [PATCH v2] ole32: Store proxy/stub CLSIDs per process, not per apartment.