[PATCH 3/3] ole32: Always grab a reference to apt in CoGetClassObject().

[Zebediah Figura]

Signed-off-by: Zebediah Figura <z.figura12 at gmail.com>
---
In a sense this is just to make the code simpler, but the problem it causes
would happen anyway. The basic problem is that destroying an apartment cleans
up its proxy manager, which calls proxy_manager_get_remunknown() in order to
release the IRemUnknown if it exists, and this may end up unmarshalling the
IRemUnknown, triggering calls to CoGetClassObject (as well as CoGetPSClsid()
etc.) Later it's also necessary to call apartment_get_current_or_mta() in
proxy_manager_get_remunknown() itself. As far as I understand we wouldn't tear
down the proxy manager from a different thread than the MTA was initialized
on, but we might call proxy_manager_get_remunknown() from a different thread,
so this is necessary.

Alternate solutions, I guess:
* Use the apartment's critical section instead. This seems existentially not
  quite appropriate, and it's not clear to me what all of the implications of
  this would be.
* Try to restructure where the apartment is grabbed.
* Perhaps don't even grab a reference to the MTA at all. This seems terrible
  on its face, but it also seems true that an app which closes the MTA while
  it's using an MTA object is pretty broken.
There is quite a lot of DCOM about, so I'd at least appreciate help in
determining whether this solution or any of the alternates is best.

 dlls/ole32/compobj.c         | 26 +++++++++++++++++---------
 dlls/ole32/compobj_private.h |  1 +
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/dlls/ole32/compobj.c b/dlls/ole32/compobj.c
index a317bf7..1c7645b 100644
--- a/dlls/ole32/compobj.c
+++ b/dlls/ole32/compobj.c
@@ -1162,9 +1162,17 @@ DWORD apartment_release(struct apartment *apt)
 
     ret = InterlockedDecrement(&apt->refs);
     TRACE("%s: after = %d\n", wine_dbgstr_longlong(apt->oxid), ret);
+
+    if (apt->being_destroyed)
+    {
+        LeaveCriticalSection(&csApartment);
+        return ret;
+    }
+
     /* destruction stuff that needs to happen under csApartment CS */
     if (ret == 0)
     {
+        apt->being_destroyed = TRUE;
         if (apt == MTA) MTA = NULL;
         else if (apt == MainApartment) MainApartment = NULL;
         list_remove(&apt->entry);
@@ -2981,7 +2989,6 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CoGetClassObject(
     IUnknown *regClassObject;
     HRESULT	hres = E_UNEXPECTED;
     APARTMENT  *apt;
-    BOOL release_apt = FALSE;
 
     TRACE("CLSID: %s,IID: %s\n", debugstr_guid(rclsid), debugstr_guid(iid));
 
@@ -2990,14 +2997,15 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CoGetClassObject(
 
     *ppv = NULL;
 
-    if (!(apt = COM_CurrentApt()))
+    if ((apt = COM_CurrentApt()))
+        apartment_addref(apt);
+    else
     {
         if (!(apt = apartment_find_mta()))
         {
             ERR("apartment not initialised\n");
             return CO_E_NOTINITIALIZED;
         }
-        release_apt = TRUE;
     }
 
     if (pServerInfo) {
@@ -3009,7 +3017,7 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CoGetClassObject(
     {
         if (IsEqualCLSID(rclsid, &CLSID_InProcFreeMarshaler))
         {
-            if (release_apt) apartment_release(apt);
+            apartment_release(apt);
             return FTMarshalCF_Create(iid, ppv);
         }
         if (IsEqualCLSID(rclsid, &CLSID_GlobalOptions))
@@ -3035,7 +3043,7 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CoGetClassObject(
 
             hres = get_inproc_class_object(apt, &clsreg, &comclass->clsid, iid, !(dwClsContext & WINE_CLSCTX_DONT_HOST), ppv);
             ReleaseActCtx(data.hActCtx);
-            if (release_apt) apartment_release(apt);
+            apartment_release(apt);
             return hres;
         }
     }
@@ -3056,7 +3064,7 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CoGetClassObject(
        * is good since we are not returning it in the "out" parameter.
        */
       IUnknown_Release(regClassObject);
-      if (release_apt) apartment_release(apt);
+      apartment_release(apt);
       return hres;
     }
 
@@ -3091,7 +3099,7 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CoGetClassObject(
          * other types */
         if (SUCCEEDED(hres))
         {
-            if (release_apt) apartment_release(apt);
+            apartment_release(apt);
             return hres;
         }
     }
@@ -3127,11 +3135,11 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CoGetClassObject(
          * other types */
         if (SUCCEEDED(hres))
         {
-            if (release_apt) apartment_release(apt);
+            apartment_release(apt);
             return hres;
         }
     }
-    if (release_apt) apartment_release(apt);
+    apartment_release(apt);
 
     /* Next try out of process */
     if (CLSCTX_LOCAL_SERVER & dwClsContext)
diff --git a/dlls/ole32/compobj_private.h b/dlls/ole32/compobj_private.h
index 9e65c3e..12413f7 100644
--- a/dlls/ole32/compobj_private.h
+++ b/dlls/ole32/compobj_private.h
@@ -142,6 +142,7 @@ struct apartment
   DWORD host_apt_tid;      /* thread ID of apartment hosting objects of differing threading model (CS cs) */
   HWND host_apt_hwnd;      /* handle to apartment window of host apartment (CS cs) */
   LocalServer *local_server; /* A marshallable object exposing local servers (CS cs) */
+  BOOL being_destroyed;    /* is currently being destroyed */
 
   /* FIXME: OIDs should be given out by RPCSS */
   OID oidc;                /* object ID counter, starts at 1, zero is invalid OID (CS cs) */
-- 
2.7.4