Rob Shearman : oleaut32: Fix circular reference counting in typelibs/ typeinfos.
Module: wine Branch: master Commit: 8a5886efadae4b9d76c45b5923c5114c9f2b042e URL: http://source.winehq.org/git/wine.git/?a=commit;h=8a5886efadae4b9d76c45b5923... Author: Rob Shearman <robertshearman(a)gmail.com> Date: Thu Nov 19 11:54:22 2009 +0000 oleaut32: Fix circular reference counting in typelibs/typeinfos. Do not rely on the reference count of ITypeInfo's to go to zero to delete them. Instead only rely on the parent typelib's reference count, but update the parent typelib's reference count based on whether each typeinfo has a valid reference. --- dlls/oleaut32/typelib.c | 173 ++++++++++++++++++++++++----------------------- 1 files changed, 88 insertions(+), 85 deletions(-) diff --git a/dlls/oleaut32/typelib.c b/dlls/oleaut32/typelib.c index bc92f5d..ee715b6 100644 --- a/dlls/oleaut32/typelib.c +++ b/dlls/oleaut32/typelib.c @@ -1048,7 +1048,7 @@ typedef struct tagITypeInfoImpl const ITypeInfo2Vtbl *lpVtbl; const ITypeCompVtbl *lpVtblTypeComp; LONG ref; - BOOL no_free_data; /* don't free data structures */ + BOOL not_attached_to_typelib; TYPEATTR TypeAttr ; /* _lots_ of type information. */ ITypeLibImpl * pTypeLib; /* back pointer to typelib */ int index; /* index in this typelib; */ @@ -1085,6 +1085,7 @@ static const ITypeInfo2Vtbl tinfvt; static const ITypeCompVtbl tcompvt; static ITypeInfo2 * ITypeInfo_Constructor(void); +static void ITypeInfo_fnDestroy(ITypeInfoImpl *This); typedef struct tagTLBContext { @@ -4089,6 +4090,7 @@ static ULONG WINAPI ITypeLib2_fnRelease( ITypeLib2 *iface) TLBRefType *ref_type; void *cursor2; int i; + ITypeInfoImpl *pTI, *pTINext; /* remove cache entry */ if(This->path) @@ -4145,8 +4147,11 @@ static ULONG WINAPI ITypeLib2_fnRelease( ITypeLib2 *iface) TLB_Free(ref_type); } - if (This->pTypeInfo) /* can be NULL */ - ITypeInfo_Release((ITypeInfo*) This->pTypeInfo); + for (pTI = This->pTypeInfo; pTI; pTI = pTINext) + { + pTINext = pTI->next; + ITypeInfo_fnDestroy(pTI); + } HeapFree(GetProcessHeap(),0,This); return 0; } @@ -4887,7 +4892,7 @@ static ITypeInfo2 * ITypeInfo_Constructor(void) { pTypeInfoImpl->lpVtbl = &tinfvt; pTypeInfoImpl->lpVtblTypeComp = &tcompvt; - pTypeInfoImpl->ref=1; + pTypeInfoImpl->ref = 0; pTypeInfoImpl->hreftype = -1; pTypeInfoImpl->TypeAttr.memidConstructor = MEMBERID_NIL; pTypeInfoImpl->TypeAttr.memidDestructor = MEMBERID_NIL; @@ -4929,97 +4934,96 @@ static ULONG WINAPI ITypeInfo_fnAddRef( ITypeInfo2 *iface) ITypeInfoImpl *This = (ITypeInfoImpl *)iface; ULONG ref = InterlockedIncrement(&This->ref); - ITypeLib2_AddRef((ITypeLib2*)This->pTypeLib); - TRACE("(%p)->ref is %u\n",This, ref); + + if (ref == 1 /* incremented from 0 */) + ITypeLib2_AddRef((ITypeLib2*)This->pTypeLib); + return ref; } -/* ITypeInfo::Release - */ -static ULONG WINAPI ITypeInfo_fnRelease(ITypeInfo2 *iface) +static void ITypeInfo_fnDestroy(ITypeInfoImpl *This) { - ITypeInfoImpl *This = (ITypeInfoImpl *)iface; - ULONG ref = InterlockedDecrement(&This->ref); + TLBFuncDesc *pFInfo, *pFInfoNext; + TLBVarDesc *pVInfo, *pVInfoNext; + TLBImplType *pImpl, *pImplNext; - TRACE("(%p)->(%u)\n",This, ref); + TRACE("destroying ITypeInfo(%p)\n",This); - if (ref) { - /* We don't release ITypeLib when ref=0 because - it means that function is called by ITypeLib2_Release */ - ITypeLib2_Release((ITypeLib2*)This->pTypeLib); - } else { - TLBFuncDesc *pFInfo, *pFInfoNext; - TLBVarDesc *pVInfo, *pVInfoNext; - TLBImplType *pImpl, *pImplNext; + SysFreeString(This->Name); + This->Name = NULL; - TRACE("destroying ITypeInfo(%p)\n",This); + SysFreeString(This->DocString); + This->DocString = NULL; - if (This->no_free_data) - goto finish_free; - - SysFreeString(This->Name); - This->Name = NULL; + SysFreeString(This->DllName); + This->DllName = NULL; - SysFreeString(This->DocString); - This->DocString = NULL; + for (pFInfo = This->funclist; pFInfo; pFInfo = pFInfoNext) + { + INT i; + for(i = 0;i < pFInfo->funcdesc.cParams; i++) + { + ELEMDESC *elemdesc = &pFInfo->funcdesc.lprgelemdescParam[i]; + if (elemdesc->u.paramdesc.wParamFlags & PARAMFLAG_FHASDEFAULT) + { + VariantClear(&elemdesc->u.paramdesc.pparamdescex->varDefaultValue); + TLB_Free(elemdesc->u.paramdesc.pparamdescex); + } + SysFreeString(pFInfo->pParamDesc[i].Name); + } + TLB_Free(pFInfo->funcdesc.lprgelemdescParam); + TLB_Free(pFInfo->pParamDesc); + TLB_FreeCustData(pFInfo->pCustData); + if (HIWORD(pFInfo->Entry) != 0 && pFInfo->Entry != (BSTR)-1) + SysFreeString(pFInfo->Entry); + SysFreeString(pFInfo->HelpString); + SysFreeString(pFInfo->Name); + + pFInfoNext = pFInfo->next; + TLB_Free(pFInfo); + } + for (pVInfo = This->varlist; pVInfo; pVInfo = pVInfoNext) + { + if (pVInfo->vardesc.varkind == VAR_CONST) + { + VariantClear(pVInfo->vardesc.u.lpvarValue); + TLB_Free(pVInfo->vardesc.u.lpvarValue); + } + TLB_FreeCustData(pVInfo->pCustData); + SysFreeString(pVInfo->Name); + pVInfoNext = pVInfo->next; + TLB_Free(pVInfo); + } + for (pImpl = This->impltypelist; pImpl; pImpl = pImplNext) + { + TLB_FreeCustData(pImpl->pCustData); + pImplNext = pImpl->next; + TLB_Free(pImpl); + } + TLB_FreeCustData(This->pCustData); - SysFreeString(This->DllName); - This->DllName = NULL; + HeapFree(GetProcessHeap(), 0, This); +} - for (pFInfo = This->funclist; pFInfo; pFInfo = pFInfoNext) - { - INT i; - for(i = 0;i < pFInfo->funcdesc.cParams; i++) - { - ELEMDESC *elemdesc = &pFInfo->funcdesc.lprgelemdescParam[i]; - if (elemdesc->u.paramdesc.wParamFlags & PARAMFLAG_FHASDEFAULT) - { - VariantClear(&elemdesc->u.paramdesc.pparamdescex->varDefaultValue); - TLB_Free(elemdesc->u.paramdesc.pparamdescex); - } - SysFreeString(pFInfo->pParamDesc[i].Name); - } - TLB_Free(pFInfo->funcdesc.lprgelemdescParam); - TLB_Free(pFInfo->pParamDesc); - TLB_FreeCustData(pFInfo->pCustData); - if (HIWORD(pFInfo->Entry) != 0 && pFInfo->Entry != (BSTR)-1) - SysFreeString(pFInfo->Entry); - SysFreeString(pFInfo->HelpString); - SysFreeString(pFInfo->Name); - - pFInfoNext = pFInfo->next; - TLB_Free(pFInfo); - } - for (pVInfo = This->varlist; pVInfo; pVInfo = pVInfoNext) - { - if (pVInfo->vardesc.varkind == VAR_CONST) - { - VariantClear(pVInfo->vardesc.u.lpvarValue); - TLB_Free(pVInfo->vardesc.u.lpvarValue); - } - TLB_FreeCustData(pVInfo->pCustData); - SysFreeString(pVInfo->Name); - pVInfoNext = pVInfo->next; - TLB_Free(pVInfo); - } - for(pImpl = This->impltypelist; pImpl; pImpl = pImplNext) - { - TLB_FreeCustData(pImpl->pCustData); - pImplNext = pImpl->next; - TLB_Free(pImpl); - } - TLB_FreeCustData(This->pCustData); +/* ITypeInfo::Release + */ +static ULONG WINAPI ITypeInfo_fnRelease(ITypeInfo2 *iface) +{ + ITypeInfoImpl *This = (ITypeInfoImpl *)iface; + ULONG ref = InterlockedDecrement(&This->ref); -finish_free: - if (This->next) - { - ITypeInfo_Release((ITypeInfo*)This->next); - } + TRACE("(%p)->(%u)\n",This, ref); - HeapFree(GetProcessHeap(),0,This); - return 0; + if (!ref) + { + BOOL not_attached_to_typelib = This->not_attached_to_typelib; + ITypeLib2_Release((ITypeLib2*)This->pTypeLib); + if (not_attached_to_typelib) + HeapFree(GetProcessHeap(), 0, This); + /* otherwise This will be freed when typelib is freed */ } + return ref; } @@ -6654,12 +6658,11 @@ static HRESULT WINAPI ITypeInfo_fnGetRefTypeInfo( *ppTInfo = (ITypeInfo*) pTypeInfoImpl; - /* we use data structures from This, so we need to keep a reference - * to it to stop it being destroyed and signal to the new instance to + /* the AddRef implicitly adds a reference to the parent typelib, which + * stops the copied data from being destroyed until the new typeinfo's + * refcount goes to zero, but we need to signal to the new instance to * not free its data structures when it is destroyed */ - pTypeInfoImpl->no_free_data = TRUE; - pTypeInfoImpl->next = This; - ITypeInfo_AddRef((ITypeInfo*) This); + pTypeInfoImpl->not_attached_to_typelib = TRUE; ITypeInfo_AddRef(*ppTInfo);
participants (1)
-
Alexandre Julliard