Bug 683802 - Coalesce type-specific cleanup indicators. r=mrbkap

This commit is contained in:
Bobby Holley 2011-09-25 15:38:01 +01:00
parent c6981dfa25
commit 53f973b84d
4 changed files with 118 additions and 104 deletions

View File

@ -350,11 +350,16 @@ txParamArrayHolder::~txParamArrayHolder()
PRUint8 i;
for (i = 0; i < mCount; ++i) {
nsXPTCVariant &variant = mArray[i];
if (variant.IsValInterface()) {
static_cast<nsISupports*>(variant.val.p)->Release();
}
else if (variant.IsValDOMString()) {
delete (nsAString*)variant.val.p;
if (variant.DoesValNeedCleanup()) {
if (variant.type.TagPart() == nsXPTType::T_DOMSTRING)
delete (nsAString*)variant.val.p;
else {
NS_ABORT_IF_FALSE(variant.type.TagPart() == nsXPTType::T_INTERFACE ||
variant.type.TagPart() == nsXPTType::T_INTERFACE_IS,
"We only support cleanup of strings and interfaces "
"here, and this looks like neither!");
static_cast<nsISupports*>(variant.val.p)->Release();
}
}
}
}
@ -419,7 +424,7 @@ txXPCOMExtensionFunctionCall::evaluate(txIEvalContext* aContext,
nsXPTCVariant &invokeParam = invokeParams[0];
invokeParam.type = paramInfo.GetType();
invokeParam.SetValIsInterface();
invokeParam.SetValNeedsCleanup();
NS_ADDREF((txIFunctionEvaluationContext*&)invokeParam.val.p = context);
// Skip first argument, since it's the context.
@ -467,7 +472,7 @@ txXPCOMExtensionFunctionCall::evaluate(txIEvalContext* aContext,
rv = adaptor->Init();
NS_ENSURE_SUCCESS(rv, rv);
invokeParam.SetValIsInterface();
invokeParam.SetValNeedsCleanup();
nodeSet.swap((txINodeSet*&)invokeParam.val.p);
break;
}
@ -497,7 +502,7 @@ txXPCOMExtensionFunctionCall::evaluate(txIEvalContext* aContext,
rv = expr->evaluateToString(aContext, *value);
NS_ENSURE_SUCCESS(rv, rv);
invokeParam.SetValIsDOMString();
invokeParam.SetValNeedsCleanup();
invokeParam.val.p = value;
break;
}
@ -513,7 +518,7 @@ txXPCOMExtensionFunctionCall::evaluate(txIEvalContext* aContext,
return NS_ERROR_OUT_OF_MEMORY;
}
invokeParam.SetValIsInterface();
invokeParam.SetValNeedsCleanup();
adaptor.swap((txIXPathObject*&)invokeParam.val.p);
break;
}
@ -540,13 +545,13 @@ txXPCOMExtensionFunctionCall::evaluate(txIEvalContext* aContext,
return NS_ERROR_FAILURE;
}
returnParam.SetValIsDOMString();
returnParam.SetValNeedsCleanup();
returnParam.val.p = value;
}
else {
returnParam.SetIndirect();
if (returnType == eNODESET || returnType == eOBJECT) {
returnParam.SetValIsInterface();
returnParam.SetValNeedsCleanup();
}
}

View File

@ -555,27 +555,27 @@ XPCVariant::VariantDataToJS(XPCLazyCallContext& lccx,
if(NS_FAILED(variant->GetAsString((char**)&xpctvar.val.p)))
return JS_FALSE;
xpctvar.type = (uint8)(TD_PSTRING | XPT_TDP_POINTER);
xpctvar.SetValIsAllocated();
xpctvar.SetValNeedsCleanup();
break;
case nsIDataType::VTYPE_STRING_SIZE_IS:
if(NS_FAILED(variant->GetAsStringWithSize(&size,
(char**)&xpctvar.val.p)))
return JS_FALSE;
xpctvar.type = (uint8)(TD_PSTRING_SIZE_IS | XPT_TDP_POINTER);
xpctvar.SetValIsAllocated();
xpctvar.SetValNeedsCleanup();
break;
case nsIDataType::VTYPE_WCHAR_STR:
if(NS_FAILED(variant->GetAsWString((PRUnichar**)&xpctvar.val.p)))
return JS_FALSE;
xpctvar.type = (uint8)(TD_PWSTRING | XPT_TDP_POINTER);
xpctvar.SetValIsAllocated();
xpctvar.SetValNeedsCleanup();
break;
case nsIDataType::VTYPE_WSTRING_SIZE_IS:
if(NS_FAILED(variant->GetAsWStringWithSize(&size,
(PRUnichar**)&xpctvar.val.p)))
return JS_FALSE;
xpctvar.type = (uint8)(TD_PWSTRING_SIZE_IS | XPT_TDP_POINTER);
xpctvar.SetValIsAllocated();
xpctvar.SetValNeedsCleanup();
break;
case nsIDataType::VTYPE_INTERFACE:
case nsIDataType::VTYPE_INTERFACE_IS:
@ -589,7 +589,7 @@ XPCVariant::VariantDataToJS(XPCLazyCallContext& lccx,
xpctvar.type = (uint8)(TD_INTERFACE_IS_TYPE | XPT_TDP_POINTER);
if(xpctvar.val.p)
xpctvar.SetValIsInterface();
xpctvar.SetValNeedsCleanup();
break;
}
case nsIDataType::VTYPE_ARRAY:
@ -710,10 +710,15 @@ VARIANT_DONE:
&iid, pErr);
}
if(xpctvar.IsValAllocated())
nsMemory::Free((char*)xpctvar.val.p);
else if(xpctvar.IsValInterface())
((nsISupports*)xpctvar.val.p)->Release();
// We may have done something in the above code that requires cleanup.
if (xpctvar.DoesValNeedCleanup())
{
if (type == nsIDataType::VTYPE_INTERFACE ||
type == nsIDataType::VTYPE_INTERFACE_IS)
((nsISupports*)xpctvar.val.p)->Release();
else
nsMemory::Free((char*)xpctvar.val.p);
}
return success;
}

View File

@ -2240,6 +2240,8 @@ class CallMethodHelper
JS_ALWAYS_INLINE JSBool ConvertIndependentParam(uint8 i);
JS_ALWAYS_INLINE JSBool ConvertDependentParams();
JS_ALWAYS_INLINE void CleanupParam(nsXPTCMiniVariant& param, nsXPTType& type);
JS_ALWAYS_INLINE JSBool HandleDipperParam(nsXPTCVariant* dp,
const nsXPTParamInfo& paramInfo);
@ -2391,69 +2393,49 @@ CallMethodHelper::~CallMethodHelper()
for(uint8 i = 0; i < paramCount; i++)
{
nsXPTCVariant* dp = GetDispatchParam(i);
const nsXPTParamInfo& paramInfo = mMethodInfo->GetParam(i);
if(dp->IsValArray())
if(paramInfo.GetType().IsArray())
{
void* p = dp->val.p;
if(!p)
continue;
// going to have to cleanup the array and perhaps its contents
if(dp->IsValAllocated() || dp->IsValInterface())
// Clean up the array contents if necessary.
if(dp->DoesValNeedCleanup())
{
// we need to figure out how many elements are present.
// We need some basic information to properly destroy the array.
JSUint32 array_count;
if(!GetArraySizeFromParam(i, &array_count))
nsXPTType datum_type;
if(!GetArraySizeFromParam(i, &array_count) ||
!NS_SUCCEEDED(mIFaceInfo->GetTypeForParam(mVTableIndex,
&paramInfo,
1, &datum_type)))
{
NS_ERROR("failed to get array length, we'll leak here");
// XXXbholley - I'm not convinced that the above calls will
// ever fail.
NS_ERROR("failed to get array information, we'll leak here");
continue;
}
if(dp->IsValAllocated())
// Loop over the array contents. For each one, we create a
// dummy 'val' and pass it to the cleanup helper.
for(JSUint32 k = 0; k < array_count; k++)
{
void** a = (void**)p;
for(JSUint32 k = 0; k < array_count; k++)
{
void* o = a[k];
if(o) nsMemory::Free(o);
}
}
else // if(dp->IsValInterface())
{
nsISupports** a = (nsISupports**)p;
for(JSUint32 k = 0; k < array_count; k++)
{
nsISupports* o = a[k];
NS_IF_RELEASE(o);
}
nsXPTCMiniVariant v;
v.val.p = static_cast<void**>(p)[k];
CleanupParam(v, datum_type);
}
}
// always free the array itself
nsMemory::Free(p);
}
else
{
if(dp->IsValJSRoot())
{
NS_ASSERTION(!dp->IsValAllocated() && !dp->IsValInterface(),
"jsvals are their own class of values");
JS_RemoveValueRoot(mCallContext, (jsval*)dp->ptr);
continue;
}
void* p = dp->val.p;
if(!p)
continue;
if(dp->IsValAllocated())
nsMemory::Free(p);
else if(dp->IsValInterface())
((nsISupports*)p)->Release();
else if(dp->IsValDOMString())
mCallContext.DeleteString((nsAString*)p);
else if(dp->IsValUTF8String())
delete (nsCString*) p;
else if(dp->IsValCString())
delete (nsCString*) p;
// Clean up single parameters (if requested).
if (dp->DoesValNeedCleanup())
CleanupParam(*dp, dp->type);
}
}
}
@ -2836,7 +2818,7 @@ CallMethodHelper::ConvertIndependentParam(uint8 i)
if(type_tag == nsXPTType::T_INTERFACE)
{
dp->SetValIsInterface();
dp->SetValNeedsCleanup();
}
jsval src;
@ -2852,7 +2834,7 @@ CallMethodHelper::ConvertIndependentParam(uint8 i)
dp->val.j = JSVAL_VOID;
if (!JS_AddValueRoot(mCallContext, &dp->val.j))
return JS_FALSE;
dp->SetValIsJSRoot();
dp->SetValNeedsCleanup();
}
if(paramInfo.IsOut())
@ -2862,7 +2844,7 @@ CallMethodHelper::ConvertIndependentParam(uint8 i)
!paramInfo.IsShared())
{
useAllocator = JS_TRUE;
dp->SetValIsAllocated();
dp->SetValNeedsCleanup();
}
if(!paramInfo.IsIn())
@ -2875,11 +2857,11 @@ CallMethodHelper::ConvertIndependentParam(uint8 i)
switch(type_tag)
{
case nsXPTType::T_IID:
dp->SetValIsAllocated();
dp->SetValNeedsCleanup();
useAllocator = JS_TRUE;
break;
case nsXPTType::T_CHAR_STR:
dp->SetValIsAllocated();
dp->SetValNeedsCleanup();
useAllocator = JS_TRUE;
break;
case nsXPTType::T_ASTRING:
@ -2890,14 +2872,14 @@ CallMethodHelper::ConvertIndependentParam(uint8 i)
// Is an 'in' DOMString. Set 'useAllocator' to indicate
// that JSData2Native should allocate a new
// nsAString.
dp->SetValIsDOMString();
dp->SetValNeedsCleanup();
useAllocator = JS_TRUE;
break;
case nsXPTType::T_UTF8STRING:
// Fall through to the C string case for now...
case nsXPTType::T_CSTRING:
dp->SetValIsCString();
dp->SetValNeedsCleanup();
useAllocator = JS_TRUE;
break;
}
@ -2968,8 +2950,6 @@ CallMethodHelper::ConvertDependentParams()
if(isArray)
{
dp->SetValIsArray();
if(NS_FAILED(mIFaceInfo->GetTypeForParam(mVTableIndex, &paramInfo, 1,
&datum_type)))
{
@ -2982,7 +2962,7 @@ CallMethodHelper::ConvertDependentParams()
if(datum_type.IsInterfacePointer())
{
dp->SetValIsInterface();
dp->SetValNeedsCleanup();
}
jsval src;
@ -2997,7 +2977,7 @@ CallMethodHelper::ConvertDependentParams()
(isArray || !paramInfo.IsShared()))
{
useAllocator = JS_TRUE;
dp->SetValIsAllocated();
dp->SetValNeedsCleanup();
}
if(!paramInfo.IsIn())
@ -3015,7 +2995,7 @@ CallMethodHelper::ConvertDependentParams()
(isArray && datum_type.TagPart() == nsXPTType::T_CHAR_STR))
{
useAllocator = JS_TRUE;
dp->SetValIsAllocated();
dp->SetValNeedsCleanup();
}
}
@ -3075,6 +3055,45 @@ CallMethodHelper::ConvertDependentParams()
return JS_TRUE;
}
// Performs all necessary teardown on a parameter after method invocation.
//
// This method should only be called if the value in question was flagged
// for cleanup (ie, if dp->DoesValNeedCleanup()).
void
CallMethodHelper::CleanupParam(nsXPTCMiniVariant& param, nsXPTType& type)
{
// We handle array elements, but not the arrays themselves.
NS_ABORT_IF_FALSE(type.TagPart() != nsXPTType::T_ARRAY, "Can't handle arrays.");
// Pointers may sometimes be null even if cleanup was requested. Combine
// the null checking for all the different types into one check here.
if (type.TagPart() != nsXPTType::T_JSVAL && param.val.p == nsnull)
return;
switch(type.TagPart())
{
case nsXPTType::T_JSVAL:
JS_RemoveValueRoot(mCallContext, (jsval*)&param.val);
break;
case nsXPTType::T_INTERFACE:
case nsXPTType::T_INTERFACE_IS:
((nsISupports*)param.val.p)->Release();
break;
case nsXPTType::T_ASTRING:
case nsXPTType::T_DOMSTRING:
mCallContext.DeleteString((nsAString*)param.val.p);
break;
case nsXPTType::T_UTF8STRING:
case nsXPTType::T_CSTRING:
delete (nsCString*) param.val.p;
break;
default:
NS_ABORT_IF_FALSE(type.IsPointer(), "Cleanup requested on unexpected type.");
nsMemory::Free(param.val.p);
break;
}
}
// Handle parameters with dipper types.
//
// Dipper types are one of the more inscrutable aspects of xpidl. In a
@ -3117,15 +3136,9 @@ CallMethodHelper::HandleDipperParam(nsXPTCVariant* dp,
// ASTRING and DOMSTRING are very similar, and both use nsAutoString.
// UTF8_STRING and CSTRING are also quite similar, and both use nsCString.
if(type_tag == nsXPTType::T_ASTRING || type_tag == nsXPTType::T_DOMSTRING)
{
dp->SetValIsDOMString();
dp->val.p = new nsAutoString();
}
else
{
dp->SetValIsCString();
dp->val.p = new nsCString();
}
// Check for OOM, in either case.
if(!dp->val.p)
@ -3134,6 +3147,9 @@ CallMethodHelper::HandleDipperParam(nsXPTCVariant* dp,
return JS_FALSE;
}
// We allocated, so we need to deallocate after the method call completes.
dp->SetValNeedsCleanup();
return JS_TRUE;
}

View File

@ -114,33 +114,21 @@ struct nsXPTCVariant : public nsXPTCMiniVariant
// the way they are.
PTR_IS_DATA = 0x1,
VAL_IS_ALLOCD = 0x2, // val.p holds alloc'd ptr that must be freed
VAL_IS_IFACE = 0x4, // val.p holds interface ptr that must be released
VAL_IS_ARRAY = 0x8, // val.p holds a pointer to an array needing cleanup
VAL_IS_DOMSTR = 0x10, // val.p holds a pointer to domstring needing cleanup
VAL_IS_UTF8STR = 0x20, // val.p holds a pointer to utf8string needing cleanup
VAL_IS_CSTR = 0x40, // val.p holds a pointer to cstring needing cleanup
VAL_IS_JSROOT = 0x80 // val.p holds a pointer to a jsval that must be unrooted
// Indicates that the value we hold requires some sort of cleanup (memory
// deallocation, interface release, jsval unrooting, etc). The precise
// cleanup that is performed depends on the 'type' field above.
// If the value is an array, this flag specifies whether the elements
// within the array require cleanup (we always clean up the array itself,
// so this flag would be redundant for that purpose).
VAL_NEEDS_CLEANUP = 0x2
};
void ClearFlags() {flags = 0;}
void SetIndirect() {ptr = &val; flags |= PTR_IS_DATA;}
void SetValIsAllocated() {flags |= VAL_IS_ALLOCD;}
void SetValIsInterface() {flags |= VAL_IS_IFACE;}
void SetValIsArray() {flags |= VAL_IS_ARRAY;}
void SetValIsDOMString() {flags |= VAL_IS_DOMSTR;}
void SetValIsUTF8String() {flags |= VAL_IS_UTF8STR;}
void SetValIsCString() {flags |= VAL_IS_CSTR;}
void SetValIsJSRoot() {flags |= VAL_IS_JSROOT;}
void SetValNeedsCleanup() {flags |= VAL_NEEDS_CLEANUP;}
PRBool IsIndirect() const {return 0 != (flags & PTR_IS_DATA);}
PRBool IsValAllocated() const {return 0 != (flags & VAL_IS_ALLOCD);}
PRBool IsValInterface() const {return 0 != (flags & VAL_IS_IFACE);}
PRBool IsValArray() const {return 0 != (flags & VAL_IS_ARRAY);}
PRBool IsValDOMString() const {return 0 != (flags & VAL_IS_DOMSTR);}
PRBool IsValUTF8String() const {return 0 != (flags & VAL_IS_UTF8STR);}
PRBool IsValCString() const {return 0 != (flags & VAL_IS_CSTR);}
PRBool IsValJSRoot() const {return 0 != (flags & VAL_IS_JSROOT);}
PRBool IsIndirect() const {return 0 != (flags & PTR_IS_DATA);}
PRBool DoesValNeedCleanup() const {return 0 != (flags & VAL_NEEDS_CLEANUP);}
// Internal use only. Use IsIndirect() instead.
PRBool IsPtrData() const {return 0 != (flags & PTR_IS_DATA);}