ICU-6076 Prevent a double mutex lock when the following happens.

ucol_open -> u_cleanup -> ures_open same locale.
Notice that the collator leaked, which contained open resource bundles. The resource bundle API didn't recover very well from this experience, and a double mutex lock happens, which is hard to debug in the test framework.

This change will make it easier to test the -m option, reduce open resources while a collator is running and make it easier to segment the collator for static libraries.

X-SVN-Rev: 23104
This commit is contained in:
George Rhoten 2007-12-18 01:19:16 +00:00
parent 5bb343c749
commit 344e2283d7
8 changed files with 93 additions and 96 deletions

View File

@ -176,8 +176,8 @@ public:
Locale canonicalLocale("");
Locale currentLocale("");
result->setLocales(lkey.canonicalLocale(canonicalLocale),
LocaleUtility::initLocaleFromName(*actualReturn, currentLocale));
LocaleUtility::initLocaleFromName(*actualReturn, currentLocale);
result->setLocales(lkey.canonicalLocale(canonicalLocale), currentLocale, currentLocale);
}
return result;
}
@ -325,7 +325,7 @@ Collator* U_EXPORT2 Collator::createInstance(const Locale& desiredLocale,
// correctly already, and we don't want to overwrite it. (TODO
// remove in 3.0) [aliu]
if (*actualLoc.getName() != 0) {
result->setLocales(desiredLocale, actualLoc);
result->setLocales(desiredLocale, actualLoc, actualLoc);
}
return result;
}
@ -552,7 +552,7 @@ int32_t U_EXPORT2 Collator::getBound(const uint8_t *source,
}
void
Collator::setLocales(const Locale& /* requestedLocale */, const Locale& /* validLocale */) {
Collator::setLocales(const Locale& /* requestedLocale */, const Locale& /* validLocale */, const Locale& /*actualLocale*/) {
}
UnicodeSet *Collator::getTailoredSet(UErrorCode &status) const

View File

@ -598,18 +598,18 @@ const Locale RuleBasedCollator::getLocale(ULocDataLocaleType type, UErrorCode &s
}
void
RuleBasedCollator::setLocales(const Locale& requestedLocale, const Locale& validLocale) {
RuleBasedCollator::setLocales(const Locale& requestedLocale, const Locale& validLocale, const Locale& actualLocale) {
checkOwned();
size_t rlen = uprv_strlen(requestedLocale.getName());
char* rloc = (char *)uprv_malloc((rlen+1)*sizeof(char));
char* rloc = uprv_strdup(requestedLocale.getName());
if (rloc) {
uprv_strcpy(rloc, requestedLocale.getName());
size_t vlen = uprv_strlen(validLocale.getName());
char* vloc = (char*)uprv_malloc((vlen+1)*sizeof(char));
char* vloc = uprv_strdup(validLocale.getName());
if (vloc) {
uprv_strcpy(vloc, validLocale.getName());
ucol_setReqValidLocales(ucollator, rloc, vloc);
return;
char* aloc = uprv_strdup(actualLocale.getName());
if (aloc) {
ucol_setReqValidLocales(ucollator, rloc, vloc, aloc);
return;
}
uprv_free(vloc);
}
uprv_free(rloc);
}

View File

@ -391,13 +391,13 @@ ucol_initFromBinary(const uint8_t *bin, int32_t length,
}
result->freeImageOnClose = FALSE;
}
result->actualLocale = NULL;
result->validLocale = NULL;
result->requestedLocale = NULL;
result->rules = NULL;
result->rulesLength = 0;
result->freeRulesOnClose = FALSE;
result->rb = NULL;
result->elements = NULL;
result->ucaRules = NULL;
return result;
}
@ -500,10 +500,11 @@ ucol_safeClone(const UCollator *coll, void *stackBuffer, int32_t * pBufferSize,
for(i = 0; i < UCOL_ATTRIBUTE_COUNT; i++) {
ucol_setAttribute(localCollator, (UColAttribute)i, ucol_getAttribute(coll, (UColAttribute)i, status), status);
}
localCollator->requestedLocale = NULL; // zero copies of pointers
// zero copies of pointers
localCollator->actualLocale = NULL;
localCollator->validLocale = NULL;
localCollator->rb = NULL;
localCollator->elements = NULL;
localCollator->requestedLocale = NULL;
localCollator->ucaRules = coll->ucaRules; // There should only be one copy here.
localCollator->freeOnClose = colAllocated;
localCollator->freeImageOnClose = imageAllocated;
return localCollator;
@ -520,12 +521,12 @@ ucol_close(UCollator *coll)
if(coll->validLocale != NULL) {
uprv_free(coll->validLocale);
}
if(coll->actualLocale != NULL) {
uprv_free(coll->actualLocale);
}
if(coll->requestedLocale != NULL) {
uprv_free(coll->requestedLocale);
}
if(coll->resCleaner != NULL) {
coll->resCleaner(coll);
}
if(coll->latinOneCEs != NULL) {
uprv_free(coll->latinOneCEs);
}
@ -750,6 +751,7 @@ UCollator* ucol_initCollator(const UCATableHeader *image, UCollator *fillIn, con
result->rules = NULL;
result->rulesLength = 0;
result->freeRulesOnClose = FALSE;
/* get the version info from UCATableHeader and populate the Collator struct*/
result->dataVersion[0] = result->image->version[0]; /* UCA Builder version*/
@ -787,13 +789,12 @@ UCollator* ucol_initCollator(const UCATableHeader *image, UCollator *fillIn, con
result->latinOneRegenTable = FALSE;
result->latinOneFailed = FALSE;
result->UCA = UCA;
result->resCleaner = NULL;
ucol_updateInternalState(result, status);
/* Normally these will be set correctly later. This is the default if you use UCA or the default. */
result->rb = NULL;
result->elements = NULL;
result->ucaRules = NULL;
result->actualLocale = NULL;
result->validLocale = NULL;
result->requestedLocale = NULL;
result->hasRealData = FALSE; // real data lives in .dat file...
@ -4599,7 +4600,7 @@ ucol_calcSortKey(const UCollator *coll,
uint8_t tertiary = 0;
uint8_t caseSwitch = coll->caseSwitch;
uint8_t tertiaryMask = coll->tertiaryMask;
int8_t tertiaryAddition = (int8_t)coll->tertiaryAddition;
int8_t tertiaryAddition = coll->tertiaryAddition;
uint8_t tertiaryTop = coll->tertiaryTop;
uint8_t tertiaryBottom = coll->tertiaryBottom;
uint8_t tertiaryCommon = coll->tertiaryCommon;
@ -5184,7 +5185,7 @@ ucol_calcSortKeySimpleTertiary(const UCollator *coll,
uint8_t tertiary = 0;
uint8_t caseSwitch = coll->caseSwitch;
uint8_t tertiaryMask = coll->tertiaryMask;
int8_t tertiaryAddition = (int8_t)coll->tertiaryAddition;
int8_t tertiaryAddition = coll->tertiaryAddition;
uint8_t tertiaryTop = coll->tertiaryTop;
uint8_t tertiaryBottom = coll->tertiaryBottom;
uint8_t tertiaryCommon = coll->tertiaryCommon;
@ -6602,7 +6603,7 @@ void ucol_updateInternalState(UCollator *coll, UErrorCode *status) {
if(coll->caseLevel == UCOL_ON || coll->caseFirst == UCOL_OFF) {
coll->tertiaryMask = UCOL_REMOVE_CASE;
coll->tertiaryCommon = UCOL_COMMON3_NORMAL;
coll->tertiaryAddition = UCOL_FLAG_BIT_MASK_CASE_SW_OFF;
coll->tertiaryAddition = (int8_t)UCOL_FLAG_BIT_MASK_CASE_SW_OFF; /* Should be 0x80 */
coll->tertiaryTop = UCOL_COMMON_TOP3_CASE_SW_OFF;
coll->tertiaryBottom = UCOL_COMMON_BOT3;
} else {

View File

@ -1909,8 +1909,8 @@ uprv_uca_canonicalClosure(tempUCATable *t,
uprv_uca_closeTempTable(tempTable);
if(U_SUCCESS(*status)) {
tempColl->rb = NULL;
tempColl->elements = NULL;
tempColl->ucaRules = NULL;
tempColl->actualLocale = NULL;
tempColl->validLocale = NULL;
tempColl->requestedLocale = NULL;
tempColl->hasRealData = TRUE;

View File

@ -487,7 +487,7 @@ ucol_cloneRuleData(const UCollator *coll, int32_t *length, UErrorCode *status);
* service.
*/
U_CFUNC void U_EXPORT2
ucol_setReqValidLocales(UCollator *coll, char *requestedLocaleToAdopt, char *validLocaleToAdopt);
ucol_setReqValidLocales(UCollator *coll, char *requestedLocaleToAdopt, char *validLocaleToAdopt, char *actualLocaleToAdopt);
#define UCOL_SPECIAL_FLAG 0xF0000000
#define UCOL_TAG_SHIFT 24
@ -861,13 +861,12 @@ struct UCollator {
UColOptionSet *options;
SortKeyGenerator *sortKeyGen;
uint32_t *latinOneCEs;
char* actualLocale;
char* validLocale;
char* requestedLocale;
const UChar *rules;
const UChar *ucaRules;
const UCollator *UCA;
ResourceCleaner *resCleaner;
UResourceBundle *rb;
UResourceBundle *elements;
const UCATableHeader *image;
UTrie mapping;
const uint32_t *latinOneMapping;
@ -922,7 +921,7 @@ struct UCollator {
UBool latinOneRegenTable;
UBool latinOneFailed;
int32_t tertiaryAddition; /* when switching case, we need to add or subtract different values */
int8_t tertiaryAddition; /* when switching case, we need to add or subtract different values */
uint8_t caseSwitch;
uint8_t tertiaryCommon;
uint8_t tertiaryMask;

View File

@ -45,18 +45,6 @@
U_NAMESPACE_USE
U_CDECL_BEGIN
static void U_CALLCONV
ucol_prv_closeResources(UCollator *coll) {
if(coll->rb != NULL) { /* pointing to read-only memory */
ures_close(coll->rb);
}
if(coll->elements != NULL) {
ures_close(coll->elements);
}
}
U_CDECL_END
/****************************************************************************/
/* Following are the open/close functions */
/* */
@ -76,6 +64,7 @@ U_CFUNC UCollator*
ucol_open_internal(const char *loc,
UErrorCode *status)
{
UErrorCode intStatus = U_ZERO_ERROR;
const UCollator* UCA = ucol_initUCA(status);
/* New version */
@ -93,7 +82,7 @@ ucol_open_internal(const char *loc,
// if there is a keyword, we pick it up and try to get elements
if(!uloc_getKeywordValue(loc, "collation", keyBuffer, 256, status)) {
// no keyword. we try to find the default setting, which will give us the keyword value
UErrorCode intStatus = U_ZERO_ERROR;
intStatus = U_ZERO_ERROR;
// finding default value does not affect collation fallback status
UResourceBundle *defaultColl = ures_getByKeyWithFallback(collations, "default", NULL, &intStatus);
if(U_SUCCESS(intStatus)) {
@ -107,7 +96,8 @@ ucol_open_internal(const char *loc,
}
ures_close(defaultColl);
}
collElem = ures_getByKeyWithFallback(collations, keyBuffer, collElem, status);
collElem = ures_getByKeyWithFallback(collations, keyBuffer, collations, status);
collations = NULL; // We just reused the collations object as collElem.
UResourceBundle *binary = NULL;
@ -115,26 +105,27 @@ ucol_open_internal(const char *loc,
*status = U_USING_DEFAULT_WARNING;
result = ucol_initCollator(UCA->image, result, UCA, status);
// if we use UCA, real locale is root
result->rb = ures_open(U_ICUDATA_COLL, "", status);
result->elements = ures_open(U_ICUDATA_COLL, "", status);
ures_close(b);
b = ures_open(U_ICUDATA_COLL, "", status);
ures_close(collElem);
collElem = ures_open(U_ICUDATA_COLL, "", status);
if(U_FAILURE(*status)) {
goto clean;
}
ures_close(b);
result->hasRealData = FALSE;
} else if(U_SUCCESS(*status)) {
int32_t len = 0;
UErrorCode binaryStatus = U_ZERO_ERROR;
intStatus = U_ZERO_ERROR;
binary = ures_getByKey(collElem, "%%CollationBin", NULL, &binaryStatus);
binary = ures_getByKey(collElem, "%%CollationBin", NULL, &intStatus);
if(binaryStatus == U_MISSING_RESOURCE_ERROR) { /* we didn't find the binary image, we should use the rules */
if(intStatus == U_MISSING_RESOURCE_ERROR) { /* we didn't find the binary image, we should use the rules */
binary = NULL;
result = tryOpeningFromRules(collElem, status);
if(U_FAILURE(*status)) {
goto clean;
}
} else if(U_SUCCESS(*status)) { /* otherwise, we'll pick a collation data that exists */
int32_t len = 0;
const uint8_t *inData = ures_getBinary(binary, &len, status);
UCATableHeader *colData = (UCATableHeader *)inData;
if(uprv_memcmp(colData->UCAVersion, UCA->image->UCAVersion, sizeof(UVersionInfo)) != 0 ||
@ -164,39 +155,48 @@ ucol_open_internal(const char *loc,
result->freeImageOnClose = FALSE;
}
}
result->rb = b;
result->elements = collElem;
len = 0;
binaryStatus = U_ZERO_ERROR;
result->rules = ures_getStringByKey(result->elements, "Sequence", &len, &binaryStatus);
result->rulesLength = len;
intStatus = U_ZERO_ERROR;
result->rules = ures_getStringByKey(collElem, "Sequence", &result->rulesLength, &intStatus);
result->freeRulesOnClose = FALSE;
} else { /* There is another error, and we're just gonna clean up */
goto clean;
}
result->validLocale = NULL; // default is to use rb info
intStatus = U_ZERO_ERROR;
result->ucaRules = ures_getStringByKey(b,"UCARules",NULL,&intStatus);
if(loc == NULL) {
loc = ures_getLocale(result->rb, status);
loc = ures_getLocale(b, status);
}
result->requestedLocale = (char *)uprv_malloc((uprv_strlen(loc)+1)*sizeof(char));
result->requestedLocale = uprv_strdup(loc);
/* test for NULL */
if (result->requestedLocale == NULL) {
*status = U_MEMORY_ALLOCATION_ERROR;
goto clean;
}
uprv_strcpy(result->requestedLocale, loc);
loc = ures_getLocale(collElem, status);
result->actualLocale = uprv_strdup(loc);
/* test for NULL */
if (result->actualLocale == NULL) {
*status = U_MEMORY_ALLOCATION_ERROR;
goto clean;
}
loc = ures_getLocale(b, status);
result->validLocale = uprv_strdup(loc);
/* test for NULL */
if (result->validLocale == NULL) {
*status = U_MEMORY_ALLOCATION_ERROR;
goto clean;
}
ures_close(b);
ures_close(collElem);
ures_close(binary);
ures_close(collations); //??? we have to decide on that. Probably affects something :)
result->resCleaner = ucol_prv_closeResources;
return result;
clean:
ures_close(b);
ures_close(collElem);
ures_close(collations);
ures_close(binary);
return NULL;
}
@ -344,8 +344,8 @@ ucol_openRules( const UChar *rules,
result->rulesLength = rulesLength;
result->freeRulesOnClose = TRUE;
}
result->rb = NULL;
result->elements = NULL;
result->ucaRules = NULL;
result->actualLocale = NULL;
result->validLocale = NULL;
result->requestedLocale = NULL;
ucol_setAttribute(result, UCOL_STRENGTH, strength, status);
@ -377,8 +377,12 @@ ucol_getRulesEx(const UCollator *coll, UColRuleOption delta, UChar *buffer, int3
if(delta == UCOL_FULL_RULES) {
/* take the UCA rules and append real rules at the end */
/* UCA rules will be probably coming from the root RB */
ucaRules = ures_getStringByKey(coll->rb,"UCARules",&UCAlen,&status);
ucaRules = coll->ucaRules;
if (ucaRules) {
UCAlen = u_strlen(ucaRules);
}
/*
ucaRules = ures_getStringByKey(coll->rb,"UCARules",&UCAlen,&status);
UResourceBundle* cresb = ures_getByKeyWithFallback(coll->rb, "collations", NULL, &status);
UResourceBundle* uca = ures_getByKeyWithFallback(cresb, "UCA", NULL, &status);
ucaRules = ures_getStringByKey(uca,"Sequence",&UCAlen,&status);
@ -643,28 +647,17 @@ ucol_getLocaleByType(const UCollator *coll, ULocDataLocaleType type, UErrorCode
UTRACE_DATA1(UTRACE_INFO, "coll=%p", coll);
switch(type) {
case ULOC_ACTUAL_LOCALE:
// validLocale is set only if service registration has explicitly set the
// requested and valid locales. if this is the case, the actual locale
// is considered to be the valid locale.
if (coll->validLocale != NULL) {
result = coll->validLocale;
} else if(coll->elements != NULL) {
result = ures_getLocale(coll->elements, status);
}
break;
case ULOC_VALID_LOCALE:
if (coll->validLocale != NULL) {
result = coll->validLocale;
} else if(coll->rb != NULL) {
result = ures_getLocale(coll->rb, status);
}
break;
case ULOC_REQUESTED_LOCALE:
result = coll->requestedLocale;
break;
default:
*status = U_ILLEGAL_ARGUMENT_ERROR;
case ULOC_ACTUAL_LOCALE:
result = coll->actualLocale;
break;
case ULOC_VALID_LOCALE:
result = coll->validLocale;
break;
case ULOC_REQUESTED_LOCALE:
result = coll->requestedLocale;
break;
default:
*status = U_ILLEGAL_ARGUMENT_ERROR;
}
UTRACE_DATA1(UTRACE_INFO, "result = %s", result);
UTRACE_EXIT_STATUS(*status);
@ -672,7 +665,7 @@ ucol_getLocaleByType(const UCollator *coll, ULocDataLocaleType type, UErrorCode
}
U_CFUNC void U_EXPORT2
ucol_setReqValidLocales(UCollator *coll, char *requestedLocaleToAdopt, char *validLocaleToAdopt)
ucol_setReqValidLocales(UCollator *coll, char *requestedLocaleToAdopt, char *validLocaleToAdopt, char *actualLocaleToAdopt)
{
if (coll) {
if (coll->validLocale) {
@ -683,6 +676,10 @@ ucol_setReqValidLocales(UCollator *coll, char *requestedLocaleToAdopt, char *val
uprv_free(coll->requestedLocale);
}
coll->requestedLocale = requestedLocaleToAdopt;
if (coll->actualLocale) {
uprv_free(coll->actualLocale);
}
coll->actualLocale = actualLocaleToAdopt;
}
}

View File

@ -920,7 +920,7 @@ protected:
* @param validLocale the valid locale
* @internal
*/
virtual void setLocales(const Locale& requestedLocale, const Locale& validLocale);
virtual void setLocales(const Locale& requestedLocale, const Locale& validLocale, const Locale& actualLocale);
public:
#if !UCONFIG_NO_SERVICE

View File

@ -805,7 +805,7 @@ protected:
* @param validLocale the valid locale
* @internal
*/
virtual void setLocales(const Locale& requestedLocale, const Locale& validLocale);
virtual void setLocales(const Locale& requestedLocale, const Locale& validLocale, const Locale& actualLocale);
private: