From a118536e31d971f025cce1818b69c695b399a05d Mon Sep 17 00:00:00 2001 From: Ram Viswanadha Date: Wed, 5 Sep 2001 22:51:53 +0000 Subject: [PATCH] ICU-1008 Fix memory leak in ISO-2022 SafeClone X-SVN-Rev: 5689 --- icu4c/source/common/ucnv.c | 20 +++++------ icu4c/source/common/ucnv2022.c | 50 ++++++++++++++++++++------- icu4c/source/common/ucnv_lmb.c | 2 +- icu4c/source/common/ucnvhz.c | 7 ++-- icu4c/source/common/ucnvisci.c | 2 +- icu4c/source/common/ucnvscsu.c | 8 +++-- icu4c/source/test/cintltst/ccapitst.c | 32 ++++++++++++++--- 7 files changed, 86 insertions(+), 35 deletions(-) diff --git a/icu4c/source/common/ucnv.c b/icu4c/source/common/ucnv.c index 0c20f2ab98..551ae5de93 100644 --- a/icu4c/source/common/ucnv.c +++ b/icu4c/source/common/ucnv.c @@ -210,7 +210,7 @@ void ucnv_close (UConverter * converter) }; UErrorCode errorCode; - if (converter == NULL || converter->isCopyLocal) + if (converter == NULL) { return; } @@ -224,16 +224,16 @@ void ucnv_close (UConverter * converter) if (converter->sharedData->impl->close != NULL) { converter->sharedData->impl->close(converter); } - - if (converter->sharedData->referenceCounter != ~0) { - umtx_lock (NULL); - if (converter->sharedData->referenceCounter != 0) { - converter->sharedData->referenceCounter--; - } - umtx_unlock (NULL); + if(!converter->isCopyLocal){ + if (converter->sharedData->referenceCounter != ~0) { + umtx_lock (NULL); + if (converter->sharedData->referenceCounter != 0) { + converter->sharedData->referenceCounter--; + } + umtx_unlock (NULL); + } + uprv_free (converter); } - uprv_free (converter); - return; } diff --git a/icu4c/source/common/ucnv2022.c b/icu4c/source/common/ucnv2022.c index f3ee2c07d0..05a00c53c6 100644 --- a/icu4c/source/common/ucnv2022.c +++ b/icu4c/source/common/ucnv2022.c @@ -93,7 +93,6 @@ typedef struct{ uint32_t version; char locale[3]; char name[30]; - }UConverterDataISO2022; /* ISO-2022 ----------------------------------------------------------------- */ @@ -673,19 +672,25 @@ _ISO2022Close(UConverter *converter) { UConverter **array = myData->myConverterArray; if (converter->extraInfo != NULL) { + if(!converter->isCopyLocal){ + /*close the array of converter pointers and free the memory*/ + while(*array!=NULL){ + if(*array==myData->currentConverter){ + myData->currentConverter=NULL; + } + ucnv_close(*array++); - /*close the array of converter pointers and free the memory*/ - while(*array!=NULL){ - if(*array==myData->currentConverter){ - myData->currentConverter=NULL; } - ucnv_close(*array++); - - } - if(myData->currentConverter){ ucnv_close(myData->currentConverter); + uprv_free (converter->extraInfo); + }else{ + /* close the currentConverter if and only if the + * ISO-2022 framework converter + */ + if(!myData->isLocaleSpecified){ + ucnv_close(myData->currentConverter); + } } - uprv_free (converter->extraInfo); } } @@ -699,6 +704,7 @@ _ISO2022Reset(UConverter *converter, UConverterResetChoice choice) { ucnv_close (myConverterData->currentConverter); myConverterData->currentConverter=NULL; } + converter->mode = UCNV_SI; } if(choice!=UCNV_RESET_TO_UNICODE) { /* re-append UTF-8 escape sequence */ @@ -949,7 +955,8 @@ T_UConverter_getNextUChar_ISO_2022(UConverterToUnicodeArgs* args, mySourceLimit = getEndOfBuffer_2022(&(args->source), args->sourceLimit, TRUE); /*Find the end of the buffer e.g : Next Escape Seq | end of Buffer*/ - if (args->converter->mode == UCNV_SO) /*Already doing some conversion*/{ + if (args->converter->mode == UCNV_SO && mySourceLimit!=args->source) + /*Already doing some conversion*/{ return ucnv_getNextUChar(myData->currentConverter, &(args->source), @@ -3263,9 +3270,9 @@ struct cloneStruct { UConverter cnv; UConverterDataISO2022 mydata; + UConverter currentCnv; /**< for ISO_2022 converter if the current converter is open */ }; - U_CFUNC UConverter * _ISO_2022_SafeClone( const UConverter *cnv, @@ -3275,6 +3282,7 @@ _ISO_2022_SafeClone( { struct cloneStruct * localClone; int32_t bufferSizeNeeded = sizeof(struct cloneStruct); + UConverterDataISO2022* cnvData = (UConverterDataISO2022*)cnv->extraInfo; if (U_FAILURE(*status)){ return 0; @@ -3287,9 +3295,25 @@ _ISO_2022_SafeClone( localClone = (struct cloneStruct *)stackBuffer; uprv_memcpy(&localClone->cnv, cnv, sizeof(UConverter)); - localClone->cnv.isCopyLocal = TRUE; uprv_memcpy(&localClone->mydata, cnv->extraInfo, sizeof(UConverterDataISO2022)); + + localClone->cnv.isCopyLocal = TRUE; + + /* clone the current converter if it is open in ISO-2022 converter and since it + * preserves conversion state in currentConverter object we need to clone it + */ + if((!cnvData->isLocaleSpecified && cnvData->currentConverter!=NULL) || + /* KR version 1 also uses the state in currentConverter for preserving state + * so we need to clone it too! + */ + (cnvData->locale[0]=='k' && cnvData->version==1)){ + + uprv_memcpy(&localClone->currentCnv, cnvData->currentConverter, sizeof(UConverter)); + + localClone->mydata.currentConverter = &localClone->currentCnv; + } + localClone->cnv.extraInfo = &localClone->mydata; return &localClone->cnv; diff --git a/icu4c/source/common/ucnv_lmb.c b/icu4c/source/common/ucnv_lmb.c index 6512939a3c..37404af56f 100644 --- a/icu4c/source/common/ucnv_lmb.c +++ b/icu4c/source/common/ucnv_lmb.c @@ -594,7 +594,7 @@ _LMBCSOpenWorker(UConverter* _this, static void _LMBCSClose(UConverter * _this) { - if (_this->extraInfo != NULL) + if (_this->extraInfo != NULL && !_this->isCopyLocal) { ulmbcs_byte_t Ix; UConverterDataLMBCS * extraInfo = (UConverterDataLMBCS *) _this->extraInfo; diff --git a/icu4c/source/common/ucnvhz.c b/icu4c/source/common/ucnvhz.c index 278a18c743..048a139a49 100644 --- a/icu4c/source/common/ucnvhz.c +++ b/icu4c/source/common/ucnvhz.c @@ -156,9 +156,10 @@ _HZOpen(UConverter *cnv, const char *name,const char *locale,uint32_t options, U } static void _HZClose(UConverter *cnv){ - - ucnv_close (((UConverterDataHZ *) (cnv->extraInfo))->gbConverter); - uprv_free(cnv->extraInfo); + if((cnv->extraInfo != NULL) && !cnv->isCopyLocal){ + ucnv_close (((UConverterDataHZ *) (cnv->extraInfo))->gbConverter); + uprv_free(cnv->extraInfo); + } } diff --git a/icu4c/source/common/ucnvisci.c b/icu4c/source/common/ucnvisci.c index 173c618a22..47c26af737 100644 --- a/icu4c/source/common/ucnvisci.c +++ b/icu4c/source/common/ucnvisci.c @@ -231,7 +231,7 @@ _ISCIIOpen(UConverter *cnv, const char *name,const char *locale,uint32_t options } static void _ISCIIClose(UConverter *cnv){ - if(cnv->extraInfo){ + if(cnv->extraInfo && !cnv->isCopyLocal){ uprv_free(cnv->extraInfo); } } diff --git a/icu4c/source/common/ucnvscsu.c b/icu4c/source/common/ucnvscsu.c index 11f72ee2cb..e22abb1156 100644 --- a/icu4c/source/common/ucnvscsu.c +++ b/icu4c/source/common/ucnvscsu.c @@ -242,9 +242,11 @@ _SCSUOpen(UConverter *cnv, U_CFUNC void _SCSUClose(UConverter *cnv) { - if(cnv->extraInfo!=NULL) { - uprv_free(cnv->extraInfo); - cnv->extraInfo=NULL; + if(!cnv->isCopyLocal){ + if(cnv->extraInfo!=NULL) { + uprv_free(cnv->extraInfo); + cnv->extraInfo=NULL; + } } } diff --git a/icu4c/source/test/cintltst/ccapitst.c b/icu4c/source/test/cintltst/ccapitst.c index 59d99aefd5..793f0d5b41 100644 --- a/icu4c/source/test/cintltst/ccapitst.c +++ b/icu4c/source/test/cintltst/ccapitst.c @@ -1084,13 +1084,19 @@ static void TestAlias() { static void TestConvertSafeClone() { -#define CLONETEST_CONVERTER_COUNT 6 +#define CLONETEST_CONVERTER_COUNT 8 - char charBuffer [10]; + char charBuffer [20]; char *pCharBuffer; const char *pConstCharBuffer; const char *charBufferLimit = charBuffer + sizeof(charBuffer)/sizeof(*charBuffer); UChar uniBuffer [] = {0x0058, 0x0059, 0x005A}; /* "XYZ" */ + UChar uniCharBuffer [20]; + char charSourceBuffer [] = { 0x1b, 0x24, 0x42 }; + char *pCharSource = charSourceBuffer; + char *pCharSourceLimit = charSourceBuffer + sizeof(charSourceBuffer); + UChar *pUCharTarget = uniCharBuffer; + UChar *pUCharTargetLimit = uniCharBuffer + sizeof(uniCharBuffer); const UChar * pUniBuffer; const UChar *uniBufferLimit = uniBuffer + sizeof(uniBuffer)/sizeof(*uniBuffer); int index; @@ -1109,7 +1115,9 @@ static void TestConvertSafeClone() someConverters[3] = ucnv_open("HZ", &err); someConverters[4] = ucnv_open("lmbcs", &err); someConverters[5] = ucnv_open("ISCII,version=0",&err); - + someConverters[6] = ucnv_open("ISO_2022,locale=kr,version=1",&err); + someConverters[7] = ucnv_open("ISO_2022,locale=jp,version=1",&err); + /* Check the various error & informational states: */ /* Null status - just returns NULL */ @@ -1195,11 +1203,27 @@ static void TestConvertSafeClone() NULL, TRUE, &err); + if(U_FAILURE(err)){ + log_err("FAIL: cloned converter failed to do fromU conversion. Error: %s\n",u_errorName(err)); + } + ucnv_toUnicode(someClonedConverters[index], + &pUCharTarget, + pUCharTargetLimit, + &pCharSource, + pCharSourceLimit, + NULL, + TRUE, + &err + ); + + if(U_FAILURE(err)){ + log_err("FAIL: cloned converter failed to do toU conversion. Error: %s\n",u_errorName(err)); + } pConstCharBuffer = charBuffer; if (uniBuffer [0] != ucnv_getNextUChar(someClonedConverters[index], &pConstCharBuffer, pCharBuffer, &err)) { - log_err("FAIL: Cloned converter failed to do conversion\n"); + log_err("FAIL: Cloned converter failed to do conversion. Error: %s\n",u_errorName(err)); } ucnv_close(someClonedConverters[index]); ucnv_close(someConverters[index]);