Mike Hearn wrote:
On Mon, 2005-01-03 at 11:47 -0600, Robert Shearman wrote:
Also, it may be best to split up the apartment structure into the stuff that really is apartment-local and the stuff that is thread-local, and then we have NtCurrentTeb()->ReservedForOle point to the thread-local structure, which has a member that points to the apartment struct.
That makes sense. I'll implement it now.
One other thing: we also need the MTA to be dynamically allocated and destroyed and so therefore also ref counted.
Rob
OK, I implemented the new OLE TLS management, and it clears up the code a lot, no more confusing apt->parent business.
I didn't make the MTA dynamically allocated because this patch already contains two changes that could really have been one given enough pain, so I added it to the todo list instead.
I think this patch is pretty much ready to go, the OLE TLS changes are straightforward enough, so I'm CCing wine-patches. Any comments on the last changes are welcome as usual.
Alexandre: this patch is against the previous patches sent, so they must be applied in order.
Mike Hearn <mh(a)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
------------------------------------------------------------------------
--- 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,16 +101,17 @@ 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; +CRITICAL_SECTION csApartment; static CRITICAL_SECTION_DEBUG critsect_debug = { 0, 0, &csApartment, { &critsect_debug.ProcessLocksList, &critsect_debug.ProcessLocksList }, 0, 0, { 0, (DWORD)(__FILE__ ": csApartment") } }; -static CRITICAL_SECTION csApartment = { &critsect_debug, -1, 0, 0, 0, 0 }; +CRITICAL_SECTION csApartment = { &critsect_debug, -1, 0, 0, 0, 0 };
I can't see the need to make this non-static at the moment.
/* * This lock count counts the number of times CoInitialize is called. It is @@ -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,138 @@ 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; + 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(APARTMENT *apt) +{ + return InterlockedIncrement(&apt->refs); +} + +/* if null, use current apt and on destroy clear the TEB apt field */ +DWORD COM_ApartmentRelease(APARTMENT *apartment)
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.
{ + APARTMENT *apt = apartment ? apartment : COM_CurrentInfo()->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)); + + if (!apartment) COM_CurrentInfo()->apt = NULL; + + 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");
To answer your question: forcefully destroy them all.
+ } + + 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 +509,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 +523,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 +538,22 @@ 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); + /* doing this twice is harmless */ + apt = COM_CreateApartment(dwCoInit);
- 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; + } + + InterlockedIncrement(&apt->refs);
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.
return hr; } @@ -529,14 +564,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 +602,10 @@ 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; - } + COM_ApartmentRelease(NULL);
/* * Decrease the reference count. @@ -1134,14 +1168,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 +1251,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 +1590,7 @@ HRESULT WINAPI CoCreateInstance( LPCLASSFACTORY lpclf = 0;
if (!COM_CurrentApt()) return CO_E_NOTINITIALIZED; - + /* * Sanity check */ @@ -1573,15 +1607,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 +2034,21 @@ HRESULT WINAPI CoInitializeWOW(DWORD x,D */ 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.
- if(apt && apt->state) { - IUnknown_AddRef(apt->state); - *ppv = apt->state; - FIXME("-- %p\n", *ppv); - return S_OK; - } - *ppv = NULL; - return E_FAIL; + *ppv = NULL; + + 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,19 @@ HRESULT WINAPI CoGetState(IUnknown ** pp */ HRESULT WINAPI CoSetState(IUnknown * pv) { - APARTMENT * apt = COM_CurrentInfo(); + FIXME("(%p),stub!\n", pv);
- 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 +2224,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 */
This should be moved into the TLS struct, not removed entirely.
DWORD tid; /* thread id */ HANDLE thread; /* thread handle */ OXID oxid; /* object exporter ID */ @@ -107,13 +105,12 @@ 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 */
You can remove this - it should no longer be needed.
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 +193,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(APARTMENT *apt); +DWORD COM_ApartmentRelease(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));
I guess are ignoring failures from HeapAlloc then. There isn't really much we can do about it anyway though.
+ + 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,8 @@ * NOTES: * * The errorinfo is a per-thread object. The reference is stored in the - * TEB at offset 0xf80 + * TEB at offset 0xf80. Does anything actually access this directly? + * If not we should make this use a different field and drop the apt->parent hack. */
Nothing should reference it directly as the offset is OS dependent.
#include <stdarg.h> @@ -483,20 +484,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 +507,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;
Rob