From b3a4b59668c0222b3bc53cba9eaf36861a7b175a Mon Sep 17 00:00:00 2001 From: Robert Shearman Date: Sun, 18 Sep 2005 11:10:37 +0000 Subject: [PATCH] Fix ref-counting rules to match native DCOM Dlls. --- dlls/ole32/marshal.c | 21 ++--- dlls/ole32/oleproxy.c | 23 +++-- dlls/ole32/tests/marshal.c | 167 +++++++++++++++++-------------------- dlls/oleaut32/tmarshal.c | 5 +- dlls/rpcrt4/cproxy.c | 5 +- 5 files changed, 110 insertions(+), 111 deletions(-) diff --git a/dlls/ole32/marshal.c b/dlls/ole32/marshal.c index f62e2b4d3d0..21d0ffe926f 100644 --- a/dlls/ole32/marshal.c +++ b/dlls/ole32/marshal.c @@ -628,6 +628,7 @@ static HRESULT proxy_manager_create_ifproxy( if (IsEqualIID(riid, &IID_IUnknown)) { ifproxy->iface = (void *)&This->lpVtbl; + IMultiQI_AddRef((IMultiQI *)&This->lpVtbl); hr = S_OK; } else @@ -702,18 +703,19 @@ static void proxy_manager_disconnect(struct proxy_manager * This) TRACE("oxid = %s, oid = %s\n", wine_dbgstr_longlong(This->oxid), wine_dbgstr_longlong(This->oid)); + EnterCriticalSection(&This->cs); + /* SORFP_NOLIFTIMEMGMT proxies (for IRemUnknown) shouldn't be * disconnected - it won't do anything anyway, except cause * problems for other objects that depend on this proxy always * working */ - if (This->sorflags & SORFP_NOLIFETIMEMGMT) return; - - EnterCriticalSection(&This->cs); - - LIST_FOR_EACH(cursor, &This->interfaces) + if (!(This->sorflags & SORFP_NOLIFETIMEMGMT)) { - struct ifproxy * ifproxy = LIST_ENTRY(cursor, struct ifproxy, entry); - ifproxy_disconnect(ifproxy); + LIST_FOR_EACH(cursor, &This->interfaces) + { + struct ifproxy * ifproxy = LIST_ENTRY(cursor, struct ifproxy, entry); + ifproxy_disconnect(ifproxy); + } } /* apartment is being destroyed so don't keep a pointer around to it */ @@ -986,12 +988,11 @@ static HRESULT unmarshal_object(const STDOBJREF *stdobjref, APARTMENT *apt, REFI hr = proxy_manager_create_ifproxy(proxy_manager, stdobjref, riid, chanbuf, &ifproxy); } + else + IUnknown_AddRef((IUnknown *)ifproxy->iface); if (hr == S_OK) - { - ClientIdentity_AddRef((IMultiQI*)&proxy_manager->lpVtbl); *object = ifproxy->iface; - } } /* release our reference to the proxy manager - the client/apartment diff --git a/dlls/ole32/oleproxy.c b/dlls/ole32/oleproxy.c index 62f5daa00f4..eda679a9c38 100644 --- a/dlls/ole32/oleproxy.c +++ b/dlls/ole32/oleproxy.c @@ -301,9 +301,8 @@ static ULONG WINAPI IRpcProxyBufferImpl_Release(LPRPCPROXYBUFFER iface) { ULONG ref = InterlockedDecrement(&This->ref); if (!ref) { - IRpcChannelBuffer_Release(This->chanbuf); - This->chanbuf = NULL; - HeapFree(GetProcessHeap(),0,This); + IRpcProxyBuffer_Disconnect(iface); + HeapFree(GetProcessHeap(),0,This); } return ref; } @@ -451,11 +450,13 @@ CFProxy_Construct(IUnknown *pUnkOuter, LPVOID *ppv,LPVOID *ppProxy) { cf->lpvtbl_cf = &cfproxyvt; cf->lpvtbl_proxy = &pspbvtbl; - /* only one reference for the proxy buffer */ + /* one reference for the proxy buffer */ cf->ref = 1; cf->outer_unknown = pUnkOuter; *ppv = &(cf->lpvtbl_cf); *ppProxy = &(cf->lpvtbl_proxy); + /* and one reference for the object */ + IUnknown_AddRef((IUnknown *)*ppv); return S_OK; } @@ -697,9 +698,15 @@ static HRESULT WINAPI RemUnkProxy_QueryInterface(LPREMUNKNOWN iface, REFIID riid static ULONG WINAPI RemUnkProxy_AddRef(LPREMUNKNOWN iface) { RemUnkProxy *This = (RemUnkProxy *)iface; + ULONG refs; TRACE("(%p)->AddRef()\n",This); - return InterlockedIncrement(&This->refs); + + if (This->outer_unknown) + refs = IUnknown_AddRef(This->outer_unknown); + else + refs = InterlockedIncrement(&This->refs); + return refs; } static ULONG WINAPI RemUnkProxy_Release(LPREMUNKNOWN iface) @@ -870,8 +877,8 @@ static ULONG WINAPI RURpcProxyBufferImpl_Release(LPRPCPROXYBUFFER iface) { ULONG ref = InterlockedDecrement(&This->refs); TRACE("%p, %ld\n", iface, ref); if (!ref) { - IRpcChannelBuffer_Release(This->chan);This->chan = NULL; - HeapFree(GetProcessHeap(),0,This); + IRpcProxyBuffer_Disconnect(iface); + HeapFree(GetProcessHeap(),0,This); } return ref; } @@ -917,6 +924,8 @@ RemUnkProxy_Construct(IUnknown *pUnkOuter, LPVOID *ppv,LPVOID *ppProxy) { This->outer_unknown = pUnkOuter; *ppv = &(This->lpvtbl_remunk); *ppProxy = &(This->lpvtbl_proxy); + /* and one reference for the object */ + IUnknown_AddRef((IUnknown *)*ppv); return S_OK; } diff --git a/dlls/ole32/tests/marshal.c b/dlls/ole32/tests/marshal.c index 72141a3267d..032b544be02 100644 --- a/dlls/ole32/tests/marshal.c +++ b/dlls/ole32/tests/marshal.c @@ -1282,6 +1282,82 @@ static void test_proxy_interfaces(void) end_host_object(tid, thread); } +typedef struct +{ + const IUnknownVtbl *lpVtbl; + ULONG refs; +} HeapUnknown; + +static HRESULT WINAPI HeapUnknown_QueryInterface(IUnknown *iface, REFIID riid, void **ppv) +{ + if (IsEqualIID(riid, &IID_IUnknown)) + { + IUnknown_AddRef(iface); + *ppv = (LPVOID)iface; + return S_OK; + } + *ppv = NULL; + return E_NOINTERFACE; +} + +static ULONG WINAPI HeapUnknown_AddRef(IUnknown *iface) +{ + HeapUnknown *This = (HeapUnknown *)iface; + trace("HeapUnknown_AddRef(%p)\n", iface); + return InterlockedIncrement((LONG*)&This->refs); +} + +static ULONG WINAPI HeapUnknown_Release(IUnknown *iface) +{ + HeapUnknown *This = (HeapUnknown *)iface; + ULONG refs = InterlockedDecrement((LONG*)&This->refs); + trace("HeapUnknown_Release(%p)\n", iface); + if (!refs) HeapFree(GetProcessHeap(), 0, This); + return refs; +} + +static const IUnknownVtbl HeapUnknown_Vtbl = +{ + HeapUnknown_QueryInterface, + HeapUnknown_AddRef, + HeapUnknown_Release +}; + +static void test_proxybuffer(REFIID riid) +{ + HRESULT hr; + IPSFactoryBuffer *psfb; + IRpcProxyBuffer *proxy; + LPVOID lpvtbl; + ULONG refs; + CLSID clsid; + HeapUnknown *pUnkOuter = (HeapUnknown *)HeapAlloc(GetProcessHeap(), 0, sizeof(*pUnkOuter)); + + pUnkOuter->lpVtbl = &HeapUnknown_Vtbl; + pUnkOuter->refs = 1; + + hr = CoGetPSClsid(riid, &clsid); + ok_ole_success(hr, CoGetPSClsid); + + hr = CoGetClassObject(&clsid, CLSCTX_INPROC_SERVER, NULL, &IID_IPSFactoryBuffer, (LPVOID*)&psfb); + ok_ole_success(hr, CoGetClassObject); + + hr = IPSFactoryBuffer_CreateProxy(psfb, (IUnknown*)pUnkOuter, riid, &proxy, &lpvtbl); + ok_ole_success(hr, IPSFactoryBuffer_CreateProxy); + ok(lpvtbl != NULL, "IPSFactoryBuffer_CreateProxy succeeded, but returned a NULL vtable!\n"); + + refs = IPSFactoryBuffer_Release(psfb); +#if 0 /* not reliable on native. maybe it leaks references! */ + ok(refs == 0, "Ref-count leak of %ld on IPSFactoryBuffer\n", refs); +#endif + + refs = IUnknown_Release((IUnknown *)lpvtbl); + ok(refs == 1, "Ref-count leak of %ld on IRpcProxyBuffer\n", refs-1); + + refs = IRpcProxyBuffer_Release(proxy); + ok(refs == 0, "Ref-count leak of %ld on IRpcProxyBuffer\n", refs); +} + static void test_stubbuffer(REFIID riid) { HRESULT hr; @@ -1428,96 +1504,6 @@ static void UnlockModuleOOP() static HWND hwnd_app; -static HRESULT WINAPI TestRE_IClassFactory_CreateInstance( - LPCLASSFACTORY iface, - LPUNKNOWN pUnkOuter, - REFIID riid, - LPVOID *ppvObj) -{ - DWORD res; - BOOL ret = SendMessageTimeout(hwnd_app, WM_NULL, 0, 0, SMTO_BLOCK, 500, &res); - todo_wine { ok(ret, "Timed out sending a message to originating window during RPC call\n"); } - return S_FALSE; -} - -static const IClassFactoryVtbl TestREClassFactory_Vtbl = -{ - Test_IClassFactory_QueryInterface, - Test_IClassFactory_AddRef, - Test_IClassFactory_Release, - TestRE_IClassFactory_CreateInstance, - Test_IClassFactory_LockServer -}; - -IClassFactory TestRE_ClassFactory = { &TestREClassFactory_Vtbl }; - -static LRESULT CALLBACK window_proc(HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam) -{ - switch (msg) - { - case WM_USER: - { - HRESULT hr; - IStream *pStream = NULL; - IClassFactory *proxy = NULL; - IUnknown *object; - DWORD tid; - HANDLE thread; - - cLocks = 0; - - hr = CreateStreamOnHGlobal(NULL, TRUE, &pStream); - ok_ole_success(hr, CreateStreamOnHGlobal); - tid = start_host_object(pStream, &IID_IClassFactory, (IUnknown*)&TestRE_ClassFactory, MSHLFLAGS_NORMAL, &thread); - - ok_more_than_one_lock(); - - IStream_Seek(pStream, ullZero, STREAM_SEEK_SET, NULL); - hr = CoUnmarshalInterface(pStream, &IID_IClassFactory, (void **)&proxy); - ok_ole_success(hr, CoReleaseMarshalData); - IStream_Release(pStream); - - ok_more_than_one_lock(); - - hr = IClassFactory_CreateInstance(proxy, NULL, &IID_IUnknown, (void **)&object); - - IClassFactory_Release(proxy); - - ok_no_locks(); - - end_host_object(tid, thread); - - PostMessage(hwnd, WM_QUIT, 0, 0); - - return 0; - } - default: - return DefWindowProc(hwnd, msg, wparam, lparam); - } -} - -static void test_message_reentrancy() -{ - WNDCLASS wndclass; - MSG msg; - - memset(&wndclass, 0, sizeof(wndclass)); - wndclass.lpfnWndProc = window_proc; - wndclass.lpszClassName = "WineCOMTest"; - RegisterClass(&wndclass); - - hwnd_app = CreateWindow("WineCOMTest", NULL, 0, CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT, NULL, NULL, NULL, 0); - ok(hwnd_app != NULL, "Window creation failed\n"); - - PostMessage(hwnd_app, WM_USER, 0, 0); - - while (GetMessage(&msg, NULL, 0, 0)) - { - TranslateMessage(&msg); - DispatchMessage(&msg); - } -} - static HRESULT WINAPI TestOOP_IClassFactory_QueryInterface( LPCLASSFACTORY iface, REFIID riid, @@ -1717,6 +1703,7 @@ START_TEST(marshal) test_bad_marshal_stream(); test_proxy_interfaces(); test_stubbuffer(&IID_IClassFactory); + test_proxybuffer(&IID_IClassFactory); test_message_reentrancy(); /* test_out_of_process_com(); */ diff --git a/dlls/oleaut32/tmarshal.c b/dlls/oleaut32/tmarshal.c index 0d4f85adc05..d615156520e 100644 --- a/dlls/oleaut32/tmarshal.c +++ b/dlls/oleaut32/tmarshal.c @@ -1967,13 +1967,14 @@ PSFacBuf_CreateProxy( } } proxy->lpvtbl2 = &tmproxyvtable; - /* 1 reference for the proxy and 1 for the object */ - proxy->ref = 2; + /* one reference for the proxy */ + proxy->ref = 1; proxy->tinfo = tinfo; memcpy(&proxy->iid,riid,sizeof(*riid)); proxy->chanbuf = 0; *ppv = (LPVOID)proxy; *ppProxy = (IRpcProxyBuffer *)&(proxy->lpvtbl2); + IUnknown_AddRef((IUnknown *)*ppv); return S_OK; } diff --git a/dlls/rpcrt4/cproxy.c b/dlls/rpcrt4/cproxy.c index 301fdf989eb..c2d71f9fa07 100644 --- a/dlls/rpcrt4/cproxy.c +++ b/dlls/rpcrt4/cproxy.c @@ -176,8 +176,8 @@ HRESULT WINAPI StdProxy_Construct(REFIID riid, This->PVtbl = vtbl->Vtbl; This->lpVtbl = &StdProxy_Vtbl; - /* 1 reference for the proxy and 1 for the object */ - This->RefCount = 2; + /* one reference for the proxy */ + This->RefCount = 1; This->stubless = stubless; This->piid = vtbl->header.piid; This->pUnkOuter = pUnkOuter; @@ -186,6 +186,7 @@ HRESULT WINAPI StdProxy_Construct(REFIID riid, This->pChannel = NULL; *ppProxy = (LPRPCPROXYBUFFER)&This->lpVtbl; *ppvObj = &This->PVtbl; + IUnknown_AddRef((IUnknown *)*ppvObj); IPSFactoryBuffer_AddRef(pPSFactory); return S_OK;