[OLE #21] Make apartment access thread safe, rework OLE TLS management (take 2)

Mike Hearn mh at codeweavers.com
Tue Jan 4 12:55:05 CST 2005


On Tue, 2005-01-04 at 12:21 -0600, Robert Shearman wrote:
> I can't see the need to make this non-static at the moment.

Right, I made it non-static when the lock was taken inside an inlined
function earlier, but I got rid of that at the end. It can go back to
being static.

> I don't really like functions like this doing special things on certain 
> input values. I don't think it is that big a deal to require the few 
> callers of this function to go to the effort of getting the apartment 
> themselves.

It was done that way so it's inside the lock, ie the TEB apt field
should be cleared before the apartment is freed, otherwise there's a
time when it's pointing to garbage.

But thinking about it, as that's TLS it probably doesn't matter. So I'll
remove this feature.

> I think it would be cleaner to put the ref-count increment inside the 
> apartment creation function. That way, no callers of the 
> COM_CreateApartment function can make the mistake of creating an 
> apartment without incrementing its refcount.

OK, done.

> > HRESULT WINAPI CoGetState(IUnknown ** ppv)
> > {
> >-	APARTMENT * apt = COM_CurrentInfo();
> >+    HRESULT    hr  = E_FAIL;
> > 
> >-	FIXME("\n");
> >+    FIXME("possible stub\n");
> >
> >  
> >
> 
> I believe the function does everything that is required of it - the only 
> problem is that we don't currently do anything with the state. I'm not 
> sure if we should though.

Do we actually have any docs on this function? I agree the "possible
stub" fixme is annoying though so I'll remove it.

> This should be moved into the TLS struct, not removed entirely.

inits has been subsumed into the refcount, each CoInitialize adds a ref.
Why is it still needed?

> >   LPVOID ErrorInfo;        /* thread error info */
> >  
> >
> 
> You can remove this - it should no longer be needed.

Right, done.

> >+    if (!NtCurrentTeb()->ReservedForOle)     
> >+        NtCurrentTeb()->ReservedForOle = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(struct oletls));
> >  
> >
> 
> I guess are ignoring failures from HeapAlloc then. There isn't really 
> much we can do about it anyway though.

Indeed, we're a bit hosed in this situation. I guess the relevant parts
of the code should return an appropriate error code for that specific
API though.

> Nothing should reference it directly as the offset is OS dependent.

Hehe, whatever :) I've seen too many "neat tricks" based on referencing
stuff at that offset. Still I took the comment out as we don't know the
format of that object anyway, so it's a bit useless ....

Here's the updated patch.

Mike Hearn <mh at codeweavers.com>
- Make apartment access thread-safe by introducing refcounting and wider
  usage of the apartment lock
- Rework OLE TLS management to eliminate uninitialised apartments and
  parent chaining
-------------- next part --------------
--- dlls/ole32/compobj.c  (revision 67)
+++ dlls/ole32/compobj.c  (local)
@@ -30,6 +30,8 @@
  *   - Rewrite the CoLockObjectExternal code, it does totally the wrong
  *     thing currently (should be controlling the stub manager)
  *
+ *   - Make the MTA dynamically allocated and refcounted
+ *
  *   - Implement the service control manager (in rpcss) to keep track
  *     of registered class objects: ISCM::ServerRegisterClsid et al
  *   - Implement the OXID resolver so we don't need magic pipe names for
@@ -99,7 +101,8 @@ static void COM_ExternalLockFreeList(voi
 
 const CLSID CLSID_StdGlobalInterfaceTable = { 0x00000323, 0, 0, {0xc0, 0, 0, 0, 0, 0, 0, 0x46} };
 
-APARTMENT MTA, *apts;
+APARTMENT MTA;
+struct list apts = LIST_INIT( apts );
 
 static CRITICAL_SECTION csApartment;
 static CRITICAL_SECTION_DEBUG critsect_debug =
@@ -236,6 +239,7 @@ static void COM_UninitMTA(void)
     MTA.oxid = 0;
 }
 
+
 /* creates an apartment structure which stores OLE thread-local
  * information. Call with COINIT_UNINITIALIZED to create an apartment
  * that will be initialized with a model later. Note: do not call
@@ -243,100 +247,134 @@ static void COM_UninitMTA(void)
  * with a different COINIT value */
 APARTMENT* COM_CreateApartment(DWORD model)
 {
-    APARTMENT *apt;
-    BOOL create = (NtCurrentTeb()->ReservedForOle == NULL);
+    APARTMENT *apt = COM_CurrentApt();
 
-    if (create)
+    if (!apt)
     {
+        if (model & COINIT_MULTITHREADED)
+        {
+            TRACE("thread 0x%lx is entering the multithreaded apartment\n", GetCurrentThreadId());
+            COM_CurrentInfo()->apt = &MTA;
+            return apt;
+        }
+
+        TRACE("creating new apartment, model=%ld\n", model);
+
         apt = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(APARTMENT));
         apt->tid = GetCurrentThreadId();
         DuplicateHandle(GetCurrentProcess(), GetCurrentThread(),
                         GetCurrentProcess(), &apt->thread,
                         THREAD_ALL_ACCESS, FALSE, 0);
-    }
-    else
-        apt = NtCurrentTeb()->ReservedForOle;
 
-    list_init(&apt->proxies);
-    list_init(&apt->stubmgrs);
-    apt->oidc = 1;
-    
-    apt->model = model;
-    if (model & COINIT_APARTMENTTHREADED) {
-      /* FIXME: how does windoze create OXIDs? */
-      apt->oxid = MTA.oxid | GetCurrentThreadId();
-      apt->win = CreateWindowA(aptWinClass, NULL, 0,
-			       0, 0, 0, 0,
-			       0, 0, OLE32_hInstance, NULL);
-      InitializeCriticalSection(&apt->cs);
-    }
-    else if (!(model & COINIT_UNINITIALIZED)) {
-      apt->parent = &MTA;
-      apt->oxid = MTA.oxid;
-    }
-    EnterCriticalSection(&csApartment);
-    if (create)
-    {
-        if (apts) apts->prev = apt;
-        apt->next = apts;
-        apts = apt;
+        list_init(&apt->stubmgrs);
+        apt->oidc = 1;
+        apt->refs = 1;
+        InitializeCriticalSection(&apt->cs);
+
+        apt->model = model;
+
+        /* we don't ref the apartment as CoInitializeEx will do it for us */
+
+        if (model & COINIT_APARTMENTTHREADED)
+        {
+            /* FIXME: how does windoze create OXIDs? */
+            apt->oxid = MTA.oxid | GetCurrentThreadId();
+            apt->win = CreateWindowA(aptWinClass, NULL, 0,
+                                     0, 0, 0, 0,
+                                     0, 0, OLE32_hInstance, NULL);
+        }
+
+        EnterCriticalSection(&csApartment);
+        list_add_head(&apts, &apt->entry);
+        LeaveCriticalSection(&csApartment);
+
+        COM_CurrentInfo()->apt = apt;
     }
-    LeaveCriticalSection(&csApartment);
-    NtCurrentTeb()->ReservedForOle = apt;
+
     return apt;
 }
 
-static void COM_DestroyApartment(APARTMENT *apt)
+DWORD COM_ApartmentAddRef(struct apartment *apt)
+{
+    return InterlockedIncrement(&apt->refs);
+}
+
+DWORD COM_ApartmentRelease(struct apartment *apt)
 {
     EnterCriticalSection(&csApartment);
-    if (apt->prev) apt->prev->next = apt->next;
-    if (apt->next) apt->next->prev = apt->prev;
-    if (apts == apt) apts = apt->next;
-    apt->prev = NULL; apt->next = NULL;
-    LeaveCriticalSection(&csApartment);
-    if (apt->model & COINIT_APARTMENTTHREADED) {
-      /* disconnect proxies to release the corresponding stubs.
-       * It is confirmed in "Essential COM" in the sub-chapter on
-       * "Lifecycle Management and Marshalling" that the native version also
-       * disconnects proxies in this function. */
-      /* FIXME: this should also be called for MTA destruction, but that
-       * requires restructuring how apartments work slightly. */
-      MARSHAL_Disconnect_Proxies(apt);
 
-      if (apt->win) DestroyWindow(apt->win);
-      DeleteCriticalSection(&apt->cs);
+    if (InterlockedDecrement(&apt->refs) == 0)
+    {
+        TRACE("destroying apartment %p, oxid %s\n", apt, wine_dbgstr_longlong(apt->oxid));
+
+        MARSHAL_Disconnect_Proxies(apt);
+
+        list_remove(&apt->entry);
+        if ((apt->model & COINIT_APARTMENTTHREADED) && apt->win) DestroyWindow(apt->win);
+
+        if (!list_empty(&apt->stubmgrs))
+        {
+            FIXME("PANIC: Apartment being destroyed with outstanding stubs, what do we do now?\n");
+        }
+
+        if (apt->filter) IUnknown_Release(apt->filter);
+
+
+        DeleteCriticalSection(&apt->cs);
+        CloseHandle(apt->thread);
+        HeapFree(GetProcessHeap(), 0, apt);
+
+        apt = NULL;
     }
-    CloseHandle(apt->thread);
-    HeapFree(GetProcessHeap(), 0, apt);
-}
 
-/* The given OXID must be local to this process: you cannot use apartment
-   windows to send RPCs to other processes. This all needs to move to rpcrt4 */
+    LeaveCriticalSection(&csApartment);
 
-APARTMENT *COM_ApartmentFromOXID(OXID oxid)
+    return apt ? apt->refs : 0;
+}
+
+/* The given OXID must be local to this process: you cannot use
+ * apartment windows to send RPCs to other processes. This all needs
+ * to move to rpcrt4.
+ *
+ * The ref parameter is here mostly to ensure people remember that
+ * they get one, you should normally take a ref for thread safety.
+ */
+APARTMENT *COM_ApartmentFromOXID(OXID oxid, BOOL ref)
 {
-    APARTMENT *apt = NULL;
+    APARTMENT *result = NULL;
+    struct list *cursor;
 
     EnterCriticalSection(&csApartment);
-    apt = apts;
-    while (apt && apt->oxid != oxid) apt = apt->next;
+    LIST_FOR_EACH( cursor, &apts )
+    {
+        struct apartment *apt = LIST_ENTRY( cursor, struct apartment, entry );
+        if (apt->oxid == oxid)
+        {
+            result = apt;
+            if (ref) COM_ApartmentAddRef(result);
+            break;
+        }
+    }
     LeaveCriticalSection(&csApartment);
 
-    return apt;
+    return result;
 }
 
-HWND COM_GetApartmentWin(OXID oxid)
+HWND COM_GetApartmentWin(OXID oxid, BOOL ref)
 {
-    APARTMENT *apt = COM_ApartmentFromOXID(oxid);
+    APARTMENT *apt;
 
-    return apt ? apt->win : NULL;
+    apt = COM_ApartmentFromOXID(oxid, ref);
+    if (!apt) return NULL;
+
+    return apt->win;
 }
 
 /* Currently inter-thread marshalling is not fully implemented, so this does nothing */
 static LRESULT CALLBACK COM_AptWndProc(HWND hWnd, UINT msg, WPARAM wParam, LPARAM lParam)
 {
   return DefWindowProcA(hWnd, msg, wParam, lParam);
-} 
+}
 
 /*****************************************************************************
  * This section contains OpenDllList implemantation
@@ -467,8 +505,7 @@ HRESULT WINAPI CoInitialize(
  *   CoUninitialize
  */
 HRESULT WINAPI CoInitializeEx(
-	LPVOID lpReserved,	/* [in] pointer to win32 malloc interface
-                                   (obsolete, should be NULL) */
+	LPVOID lpReserved,	/* [in] pointer to win32 malloc interface (obsolete, should be NULL) */
 	DWORD dwCoInit		/* [in] A value from COINIT specifies the threading model */
 )
 {
@@ -482,21 +519,6 @@ HRESULT WINAPI CoInitializeEx(
     ERR("(%p, %x) - Bad parameter passed-in %p, must be an old Windows Application\n", lpReserved, (int)dwCoInit, lpReserved);
   }
 
-  apt = NtCurrentTeb()->ReservedForOle;
-  if (apt && !(apt->model == COINIT_UNINITIALIZED))
-  {
-    if (dwCoInit != apt->model)
-    {
-      /* Changing the threading model after it's been set is illegal. If this warning is triggered by Wine
-         code then we are probably using the wrong threading model to implement that API. */
-      ERR("Attempt to change threading model of this apartment from 0x%lx to 0x%lx\n", apt->model, dwCoInit);
-      return RPC_E_CHANGED_MODE;
-    }
-    hr = S_FALSE;
-  }
-  else
-    hr = S_OK;
-
   /*
    * Check the lock count. If this is the first time going through the initialize
    * process, we have to initialize the libraries.
@@ -512,13 +534,27 @@ HRESULT WINAPI CoInitializeEx(
 
     COM_InitMTA();
 
+    /* we may need to defer this until after apartment initialisation */
     RunningObjectTableImpl_Initialize();
   }
 
-  if (!apt || apt->model == COINIT_UNINITIALIZED) apt = COM_CreateApartment(dwCoInit);
+  if (!COM_CurrentInfo()->apt)
+  {
+    apt = COM_CreateApartment(dwCoInit);
+  }
+  else
+  {
+    InterlockedIncrement(&apt->refs);
+  }
 
-  InterlockedIncrement(&apt->inits);
-  if (hr == S_OK) NtCurrentTeb()->ReservedForOle = apt;
+  if (dwCoInit != apt->model)
+  {
+    /* Changing the threading model after it's been set is illegal. If this warning is triggered by Wine
+       code then we are probably using the wrong threading model to implement that API. */
+    ERR("Attempt to change threading model of this apartment from 0x%lx to 0x%lx\n", apt->model, dwCoInit);
+    return RPC_E_CHANGED_MODE;
+  }
+  
 
   return hr;
 }
@@ -529,14 +565,20 @@ HRESULT WINAPI CoInitializeEx(
 void COM_FlushMessageQueue(void)
 {
     MSG message;
-    APARTMENT *apt = NtCurrentTeb()->ReservedForOle;
+    APARTMENT *apt = COM_CurrentApt();
 
     if (!apt || !apt->win) return;
 
     TRACE("Flushing STA message queue\n");
 
-    while (PeekMessageA(&message, NULL, 0, 0, PM_REMOVE)) {
-        if (message.hwnd != apt->win) continue;
+    while (PeekMessageA(&message, NULL, 0, 0, PM_REMOVE))
+    {
+        if (message.hwnd != apt->win)
+        {
+            WARN("discarding message 0x%x for window %p\n", message.message, message.hwnd);
+            continue;
+        }
+
         TranslateMessage(&message);
         DispatchMessageA(&message);
     }
@@ -561,17 +603,11 @@ void COM_FlushMessageQueue(void)
 void WINAPI CoUninitialize(void)
 {
   LONG lCOMRefCnt;
-  APARTMENT *apt;
 
   TRACE("()\n");
 
-  apt = NtCurrentTeb()->ReservedForOle;
-  if (!apt) return;
-  if (InterlockedDecrement(&apt->inits)==0) {
-    NtCurrentTeb()->ReservedForOle = NULL;
-    COM_DestroyApartment(apt);
-    apt = NULL;
-  }
+  if (!COM_ApartmentRelease(COM_CurrentInfo()->apt))
+      COM_CurrentInfo()->apt = NULL;
 
   /*
    * Decrease the reference count.
@@ -1134,14 +1170,14 @@ end:
 
 /******************************************************************************
  *		CoRegisterClassObject	[OLE32.@]
- * 
+ *
  * Registers the class object for a given class ID. Servers housed in EXE
  * files use this method instead of exporting DllGetClassObject to allow
  * other code to connect to their objects.
  *
  * RETURNS
- *   S_OK on success, 
- *   E_INVALIDARG if lpdwRegister or pUnk are NULL, 
+ *   S_OK on success,
+ *   E_INVALIDARG if lpdwRegister or pUnk are NULL,
  *   CO_E_OBJISREG if the object is already registered. We should not return this.
  *
  * SEE ALSO
@@ -1217,7 +1253,7 @@ HRESULT WINAPI CoRegisterClassObject(
   if (dwClsContext & CLSCTX_LOCAL_SERVER) {
       IClassFactory *classfac;
 
-      hr = IUnknown_QueryInterface(newClass->classObject, &IID_IClassFactory, 
+      hr = IUnknown_QueryInterface(newClass->classObject, &IID_IClassFactory,
                                    (LPVOID*)&classfac);
       if (hr) return hr;
 
@@ -1556,7 +1592,7 @@ HRESULT WINAPI CoCreateInstance(
   LPCLASSFACTORY lpclf = 0;
 
   if (!COM_CurrentApt()) return CO_E_NOTINITIALIZED;
-        
+
   /*
    * Sanity check
    */
@@ -1573,15 +1609,15 @@ HRESULT WINAPI CoCreateInstance(
    * Rather than create a class factory, we can just check for it here
    */
   if (IsEqualIID(rclsid, &CLSID_StdGlobalInterfaceTable)) {
-    if (StdGlobalInterfaceTableInstance == NULL) 
+    if (StdGlobalInterfaceTableInstance == NULL)
       StdGlobalInterfaceTableInstance = StdGlobalInterfaceTable_Construct();
     hres = IGlobalInterfaceTable_QueryInterface( (IGlobalInterfaceTable*) StdGlobalInterfaceTableInstance, iid, ppv);
     if (hres) return hres;
-    
+
     TRACE("Retrieved GIT (%p)\n", *ppv);
     return S_OK;
   }
-  
+
   /*
    * Get a class factory to construct the object we want.
    */
@@ -2000,19 +2036,19 @@ HRESULT WINAPI CoInitializeWOW(DWORD x,D
  */
 HRESULT WINAPI CoGetState(IUnknown ** ppv)
 {
-	APARTMENT * apt = COM_CurrentInfo();
+    HRESULT    hr  = E_FAIL;
 
-	FIXME("\n");
+    *ppv = NULL;
 
-	if(apt && apt->state) {
-	    IUnknown_AddRef(apt->state);
-	    *ppv = apt->state;
-	    FIXME("-- %p\n", *ppv);
-	    return S_OK;
-	}
-	*ppv = NULL;
-	return E_FAIL;
+    if (COM_CurrentInfo()->state)
+    {
+        IUnknown_AddRef(COM_CurrentInfo()->state);
+        *ppv = COM_CurrentInfo()->state;
+        TRACE("apt->state=%p\n", COM_CurrentInfo()->state);
+        hr = S_OK;
+    }
 
+    return hr;
 }
 
 /***********************************************************************
@@ -2021,22 +2057,17 @@ HRESULT WINAPI CoGetState(IUnknown ** pp
  */
 HRESULT WINAPI CoSetState(IUnknown * pv)
 {
-    APARTMENT * apt = COM_CurrentInfo();
-
-    if (!apt) apt = COM_CreateApartment(COINIT_UNINITIALIZED);
+    if (pv) IUnknown_AddRef(pv);
 
-	FIXME("(%p),stub!\n", pv);
+    if (COM_CurrentInfo()->state)
+    {
+        TRACE("-- release %p now\n", COM_CurrentInfo()->state);
+        IUnknown_Release(COM_CurrentInfo()->state);
+    }
 
-	if (pv) {
-	    IUnknown_AddRef(pv);
-	}
+    COM_CurrentInfo()->state = pv;
 
-	if (apt->state) {
-	    TRACE("-- release %p now\n", apt->state);
-	    IUnknown_Release(apt->state);
-	}
-	apt->state = pv;
-	return S_OK;
+    return S_OK;
 }
 
 
@@ -2191,7 +2222,7 @@ HRESULT WINAPI CoGetTreatAsClass(REFCLSI
 done:
     if (hkey) RegCloseKey(hkey);
     return res;
-    
+
 }
 
 /***********************************************************************
--- dlls/ole32/compobj_private.h  (revision 67)
+++ dlls/ole32/compobj_private.h  (local)
@@ -38,12 +38,8 @@
 #include "winreg.h"
 #include "winternl.h"
 
-/* Windows maps COINIT values to 0x80 for apartment threaded, 0x140
- * for free threaded, and 0 for uninitialized apartments. There is
- * no real advantage in us doing this and certainly no release version
- * of an app should be poking around with these flags. So we need a
- * special value for uninitialized */
-#define COINIT_UNINITIALIZED 0x100
+struct apartment;
+
 
 /* exported interface */
 typedef struct tagXIF {
@@ -59,7 +55,7 @@ typedef struct tagXIF {
 /* exported object */
 typedef struct tagXOBJECT {
   IRpcStubBufferVtbl *lpVtbl;
-  struct tagAPARTMENT *parent;
+  struct apartment *parent;
   struct tagXOBJECT *next;
   LPUNKNOWN obj;           /* object identity (IUnknown) */
   OID oid;                 /* object ID */
@@ -83,7 +79,7 @@ struct ifproxy
 struct proxy_manager
 {
   const IInternalUnknownVtbl *lpVtbl;
-  struct tagAPARTMENT *parent;
+  struct apartment *parent;
   struct list entry;
   LPRPCCHANNELBUFFER chan; /* channel to object */
   OXID oxid;               /* object exported ID */
@@ -93,11 +89,13 @@ struct proxy_manager
   CRITICAL_SECTION cs;     /* thread safety for this object and children */
 };
 
-/* apartment */
-typedef struct tagAPARTMENT {
-  struct tagAPARTMENT *next, *prev, *parent;
+/* this needs to become a COM object that implements IRemUnknown */
+struct apartment
+{
+  struct list entry;       
+
+  DWORD refs;              /* refcount of the apartment */
   DWORD model;             /* threading model */
-  DWORD inits;             /* CoInitialize count */
   DWORD tid;               /* thread id */
   HANDLE thread;           /* thread handle */
   OXID oxid;               /* object exporter ID */
@@ -107,13 +105,11 @@ typedef struct tagAPARTMENT {
   LPMESSAGEFILTER filter;  /* message filter */
   XOBJECT *objs;           /* exported objects */
   struct list proxies;     /* imported objects */
-  LPUNKNOWN state;         /* state object (see Co[Get,Set]State) */
-  LPVOID ErrorInfo;        /* thread error info */
   DWORD listenertid;       /* id of apartment_listener_thread */
   struct list stubmgrs;    /* stub managers for exported objects */
-} APARTMENT;
+};
 
-extern APARTMENT MTA, *apts;
+typedef struct apartment APARTMENT;
 
 extern void* StdGlobalInterfaceTable_Construct(void);
 extern void  StdGlobalInterfaceTable_Destroy(void* self);
@@ -196,27 +192,41 @@ int WINAPI FileMonikerImpl_DecomposePath
 
 HRESULT WINAPI __CLSIDFromStringA(LPCSTR idstr, CLSID *id);
 
+/* compobj.c */
+APARTMENT *COM_CreateApartment(DWORD model);
+APARTMENT *COM_ApartmentFromOXID(OXID oxid, BOOL ref);
+DWORD COM_ApartmentAddRef(struct apartment *apt);
+DWORD COM_ApartmentRelease(struct apartment *apt);
+
+extern CRITICAL_SECTION csApartment;
+
+/* this is what is stored in TEB->ReservedForOle */
+struct oletls
+{
+    struct apartment *apt;
+    IErrorInfo       *errorinfo;   /* see errorinfo.c */
+    IUnknown         *state;       /* see CoSetState */
+};
+
 /*
  * Per-thread values are stored in the TEB on offset 0xF80,
  * see http://www.microsoft.com/msj/1099/bugslayer/bugslayer1099.htm
  */
-static inline APARTMENT* COM_CurrentInfo(void)
+
+/* will create if necessary */
+static inline struct oletls *COM_CurrentInfo(void)
 {
-  APARTMENT* apt = NtCurrentTeb()->ReservedForOle;
-  return apt;
+    if (!NtCurrentTeb()->ReservedForOle)     
+        NtCurrentTeb()->ReservedForOle = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(struct oletls));
+    
+    return NtCurrentTeb()->ReservedForOle;
 }
+
 static inline APARTMENT* COM_CurrentApt(void)
-{
-  APARTMENT* apt = COM_CurrentInfo();
-  if (apt && apt->parent) apt = apt->parent;
-  return apt;
+{  
+    return COM_CurrentInfo()->apt;
 }
 
-/* compobj.c */
-APARTMENT *COM_CreateApartment(DWORD model);
-HWND COM_GetApartmentWin(OXID oxid);
-APARTMENT *COM_ApartmentFromOXID(OXID oxid);
-
 #define ICOM_THIS_MULTI(impl,field,iface) impl* const This=(impl*)((char*)(iface) - offsetof(impl,field))
 
 #endif /* __WINE_OLE_COMPOBJ_H */
--- dlls/ole32/errorinfo.c  (revision 67)
+++ dlls/ole32/errorinfo.c  (local)
@@ -20,7 +20,7 @@
  * NOTES:
  *
  * The errorinfo is a per-thread object. The reference is stored in the
- * TEB at offset 0xf80
+ * TEB at offset 0xf80.
  */
 
 #include <stdarg.h>
@@ -483,20 +483,20 @@ HRESULT WINAPI CreateErrorInfo(ICreateEr
  */
 HRESULT WINAPI GetErrorInfo(ULONG dwReserved, IErrorInfo **pperrinfo)
 {
-	APARTMENT * apt = COM_CurrentInfo();
-
-	TRACE("(%ld, %p, %p)\n", dwReserved, pperrinfo, COM_CurrentInfo()->ErrorInfo);
+	TRACE("(%ld, %p, %p)\n", dwReserved, pperrinfo, COM_CurrentInfo()->errorinfo);
 
 	if(!pperrinfo) return E_INVALIDARG;
-	if (!apt || !apt->ErrorInfo)
+        
+	if (!COM_CurrentInfo()->errorinfo)
 	{
 	   *pperrinfo = NULL;
 	   return S_FALSE;
 	}
 
-	*pperrinfo = (IErrorInfo*)(apt->ErrorInfo);
+	*pperrinfo = COM_CurrentInfo()->errorinfo;
+        
 	/* clear thread error state */
-	apt->ErrorInfo = NULL;
+	COM_CurrentInfo()->errorinfo = NULL;
 	return S_OK;
 }
 
@@ -506,18 +506,16 @@ HRESULT WINAPI GetErrorInfo(ULONG dwRese
 HRESULT WINAPI SetErrorInfo(ULONG dwReserved, IErrorInfo *perrinfo)
 {
 	IErrorInfo * pei;
-	APARTMENT * apt = COM_CurrentInfo();
 
 	TRACE("(%ld, %p)\n", dwReserved, perrinfo);
 	
-	if (!apt) apt = COM_CreateApartment(COINIT_UNINITIALIZED);
-
 	/* release old errorinfo */
-	pei = (IErrorInfo*)apt->ErrorInfo;
-	if(pei) IErrorInfo_Release(pei);
+	pei = COM_CurrentInfo()->errorinfo;
+	if (pei) IErrorInfo_Release(pei);
 
 	/* set to new value */
-	apt->ErrorInfo = perrinfo;
-	if(perrinfo) IErrorInfo_AddRef(perrinfo);
+	COM_CurrentInfo()->errorinfo = perrinfo;
+	if (perrinfo) IErrorInfo_AddRef(perrinfo);
+        
 	return S_OK;
 }
--- dlls/ole32/marshal.c  (revision 67)
+++ dlls/ole32/marshal.c  (local)
@@ -109,9 +109,13 @@ static HRESULT register_ifstub(wine_mars
     }
     else
     {
-        TRACE("constructing new stub manager\n");
+        struct apartment *apt;
         
-        manager = new_stub_manager(COM_ApartmentFromOXID(mid->oxid), obj);
+        TRACE("constructing new stub manager\n");
+
+        apt = COM_ApartmentFromOXID(mid->oxid, TRUE);
+        manager = new_stub_manager(apt, obj);
+        COM_ApartmentRelease(apt);
         if (!manager) return E_OUTOFMEMORY;
 
         if (!tablemarshal) stub_manager_ref(manager, 1);
--- dlls/ole32/rpc.c  (revision 67)
+++ dlls/ole32/rpc.c  (local)
@@ -308,8 +308,6 @@ PipeBuf_Release(LPRPCCHANNELBUFFER iface
     if (ref)
 	return ref;
 
-    FIXME("Free all stuff\n");
-
     memcpy(&header.mid, &This->mid, sizeof(wine_marshal_id));
 
     pipe = PIPE_FindByMID(&This->mid);
@@ -892,7 +890,7 @@ static DWORD WINAPI apartment_listener_t
     HANDLE		listenPipe;
     APARTMENT          *apt = (APARTMENT *) param;
 
-    /* we must join the marshalling threads apartment */
+    /* we must join the marshalling threads apartment. we already have a ref here */
     NtCurrentTeb()->ReservedForOle = apt;
 
     sprintf(pipefn,OLESTUBMGR"_%08lx%08lx", (DWORD)(apt->oxid >> 32), (DWORD)(apt->oxid));
@@ -926,7 +924,7 @@ static DWORD WINAPI apartment_listener_t
 void start_apartment_listener_thread()
 {
     APARTMENT *apt = COM_CurrentApt();
-
+    
     assert( apt );
     
     TRACE("apt->listenertid=%ld\n", apt->listenertid);
--- dlls/ole32/stubmanager.c  (revision 67)
+++ dlls/ole32/stubmanager.c  (local)
@@ -41,6 +41,7 @@
 
 WINE_DEFAULT_DEBUG_CHANNEL(ole);
 
+/* this refs the apartment on success, otherwise there is no ref */
 struct stub_manager *new_stub_manager(APARTMENT *apt, IUnknown *object)
 {
     struct stub_manager *sm;
@@ -71,6 +72,7 @@ struct stub_manager *new_stub_manager(AP
     list_add_head(&apt->stubmgrs, &sm->entry);
     LeaveCriticalSection(&apt->cs);
 
+    COM_ApartmentAddRef(apt);
     TRACE("Created new stub manager (oid=%s) at %p for object with IUnknown %p\n", wine_dbgstr_longlong(sm->oid), sm, object);
     
     return sm;
@@ -82,7 +84,7 @@ struct stub_manager *get_stub_manager_fr
     struct list         *cursor;
     APARTMENT           *apt;
 
-    if (!(apt = COM_ApartmentFromOXID(oxid)))
+    if (!(apt = COM_ApartmentFromOXID(oxid, TRUE)))
     {
         WARN("Could not map OXID %s to apartment object\n", wine_dbgstr_longlong(oxid));
         return NULL;
@@ -101,6 +103,8 @@ struct stub_manager *get_stub_manager_fr
     }
     LeaveCriticalSection(&apt->cs);
 
+    COM_ApartmentRelease(apt);
+    
     TRACE("found %p from object %p\n", result, object);
 
     return result;    
@@ -112,14 +116,13 @@ struct stub_manager *get_stub_manager(OX
     struct list         *cursor;
     APARTMENT           *apt;
 
-    if (!(apt = COM_ApartmentFromOXID(oxid)))
+    if (!(apt = COM_ApartmentFromOXID(oxid, TRUE)))
     {
         WARN("Could not map OXID %s to apartment object\n", wine_dbgstr_longlong(oxid));
         return NULL;
     }
     
     EnterCriticalSection(&apt->cs);
-    
     LIST_FOR_EACH( cursor, &apt->stubmgrs )
     {
         struct stub_manager *m = LIST_ENTRY( cursor, struct stub_manager, entry );
@@ -130,9 +133,10 @@ struct stub_manager *get_stub_manager(OX
             break;
         }
     }
-    
     LeaveCriticalSection(&apt->cs);
 
+    COM_ApartmentRelease(apt);
+
     TRACE("found %p from oid %s\n", result, wine_dbgstr_longlong(oid));
 
     return result;


More information about the wine-patches mailing list