ICU-8451 Fix memory leaks in region implementation

X-SVN-Rev: 33054

diff --git a/icu4c/source/i18n/region.cpp b/icu4c/source/i18n/region.cpp
index 9c3ecb4889c0f3fd51f027ed5b7b280aee934295..ea2f28022fc1f28cba7e72c2d638ca029b49cc6d 100644 (file)
--- a/icu4c/source/i18n/region.cpp
+++ b/icu4c/source/i18n/region.cpp
@@ -24,48 +24,69 @@
 #include "unicode/unistr.h"\r
 #include "unicode/ures.h"\r
 #include "unicode/decimfmt.h"\r
+#include "ucln_in.h"\r
 #include "cstring.h"\r
 #include "uhash.h"\r
+#include "umutex.h"\r
 #include "uresimp.h"\r
 #include "region_impl.h"\r
 \r
 #if !UCONFIG_NO_FORMATTING\r
 \r
-U_NAMESPACE_BEGIN\r
 \r
+\r
+static UMutex gRegionDataLock = U_MUTEX_INITIALIZER;\r
 static UBool regionDataIsLoaded = false;\r
-static UVector* regions = NULL;\r
 static UVector* availableRegions[URGN_LIMIT];\r
 \r
 static UHashtable *regionAliases;\r
 static UHashtable *regionIDMap;\r
 static UHashtable *numericCodeMap;\r
 \r
-static UnicodeString UNKNOWN_REGION_ID = UNICODE_STRING_SIMPLE("ZZ");\r
-static UnicodeString OUTLYING_OCEANIA_REGION_ID = UNICODE_STRING_SIMPLE("QO");\r
-static UnicodeString WORLD_ID = UNICODE_STRING_SIMPLE("001");\r
+U_CDECL_BEGIN\r
 \r
-UOBJECT_DEFINE_RTTI_IMPLEMENTATION(Region)\r
-UOBJECT_DEFINE_RTTI_IMPLEMENTATION(RegionNameEnumeration)\r
+static void U_CALLCONV\r
+deleteRegion(void *obj) {\r
+    delete (icu::Region *)obj;\r
+}\r
 \r
-void addRegion( Region *r ) {\r
+/**\r
+ * Cleanup callback func\r
+ */\r
+static UBool U_CALLCONV region_cleanup(void)\r
+{\r
+    for (int32_t i = 0 ; i < URGN_LIMIT ; i++ ) {\r
+        if ( availableRegions[i] ) {\r
+            delete availableRegions[i];\r
+        }\r
+    }\r
 \r
-    UErrorCode status = U_ZERO_ERROR;\r
-    if ( regions == NULL ) {\r
-        regions = new UVector(NULL, NULL, status);\r
+    if (regionAliases) {\r
+        uhash_close(regionAliases);\r
     }\r
-    regions->addElement(r,status);\r
-}\r
 \r
-void addAvailableRegion( const Region *r , URegionType type) {\r
+    if (numericCodeMap) {\r
+        uhash_close(numericCodeMap);\r
+    }\r
 \r
-    UErrorCode status = U_ZERO_ERROR;\r
-    if ( availableRegions[type] == NULL ) {\r
-        availableRegions[type] = new UVector(NULL, uhash_compareChars, status);\r
+    if (regionIDMap) {\r
+        uhash_close(regionIDMap);\r
     }\r
 \r
-    availableRegions[type]->addElement((void *)r->getRegionCode(),status);\r
+    return TRUE;\r
 }\r
+\r
+U_CDECL_END\r
+\r
+U_NAMESPACE_BEGIN\r
+\r
+static UnicodeString UNKNOWN_REGION_ID = UNICODE_STRING_SIMPLE("ZZ");\r
+static UnicodeString OUTLYING_OCEANIA_REGION_ID = UNICODE_STRING_SIMPLE("QO");\r
+static UnicodeString WORLD_ID = UNICODE_STRING_SIMPLE("001");\r
+\r
+UOBJECT_DEFINE_RTTI_IMPLEMENTATION(Region)\r
+UOBJECT_DEFINE_RTTI_IMPLEMENTATION(RegionNameEnumeration)\r
+\r
 /*\r
  * Initializes the region data from the ICU resource bundles.  The region data\r
  * contains the basic relationships such as which regions are known, what the numeric\r
@@ -79,7 +100,15 @@ void Region::loadRegionData() {
     if (regionDataIsLoaded) {\r
         return;\r
     }\r
-    \r
+\r
+    umtx_lock(&gRegionDataLock);\r
+\r
+    if (regionDataIsLoaded) { // In case another thread gets to it before we do...\r
+        umtx_unlock(&gRegionDataLock);\r
+        return;\r
+    }\r
+\r
+   \r
     UErrorCode status = U_ZERO_ERROR;\r
 \r
     UResourceBundle* regionCodes = NULL;\r
@@ -92,10 +121,14 @@ void Region::loadRegionData() {
     DecimalFormat *df = new DecimalFormat(status);\r
     df->setParseIntegerOnly(TRUE);\r
 \r
-    regionAliases = uhash_open(uhash_hashUnicodeString,uhash_compareUnicodeString,NULL,&status);\r
     regionIDMap = uhash_open(uhash_hashUnicodeString,uhash_compareUnicodeString,NULL,&status);\r
+    uhash_setValueDeleter(regionIDMap, deleteRegion);\r
+\r
     numericCodeMap = uhash_open(uhash_hashLong,uhash_compareLong,NULL,&status);\r
 \r
+    regionAliases = uhash_open(uhash_hashUnicodeString,uhash_compareUnicodeString,NULL,&status);\r
+    uhash_setKeyDeleter(regionAliases,uprv_deleteUObject);\r
+\r
     UResourceBundle *rb = ures_openDirect(NULL,"metadata",&status);\r
     regionCodes = ures_getByKey(rb,"regionCodes",NULL,&status);\r
     territoryAlias = ures_getByKey(rb,"territoryAlias",NULL,&status);\r
@@ -138,7 +171,6 @@ void Region::loadRegionData() {
         } else {\r
             r->code = Region::UNDEFINED_NUMERIC_CODE;\r
         }\r
-        addRegion(r);\r
     }\r
 \r
 \r
@@ -171,13 +203,12 @@ void Region::loadRegionData() {
                     aliasFromRegion->code = Region::UNDEFINED_NUMERIC_CODE;\r
                 }\r
                 aliasFromRegion->type = URGN_DEPRECATED;\r
-                addRegion(aliasFromRegion);\r
             } else {\r
                 aliasFromRegion->type = URGN_DEPRECATED;\r
             }\r
             delete aliasFromStr;\r
 \r
-            aliasFromRegion->preferredValues = new UVector(NULL, uhash_compareChars, status);\r
+            aliasFromRegion->preferredValues = new UVector(uprv_deleteUObject, uhash_compareUnicodeString, status);\r
             UnicodeString currentRegion;\r
             currentRegion.remove();\r
             for (int32_t i = 0 ; i < aliasTo.length() ; i++ ) {\r
@@ -187,7 +218,8 @@ void Region::loadRegionData() {
                 if ( aliasTo.charAt(i) == 0x0020 || i+1 == aliasTo.length() ) {\r
                     Region *target = (Region *)uhash_get(regionIDMap,(void *)&currentRegion);\r
                     if (target) {\r
-                        aliasFromRegion->preferredValues->addElement((void *)target->id,status);\r
+                        UnicodeString *preferredValue = new UnicodeString(target->idStr);\r
+                        aliasFromRegion->preferredValues->addElement((void *)preferredValue,status);\r
                     }\r
                     currentRegion.remove();\r
                 }\r
@@ -269,9 +301,12 @@ void Region::loadRegionData() {
 \r
                 // Add the child region to the set of regions contained by the parent\r
                 if (parentRegion->containedRegions == NULL) {\r
-                    parentRegion->containedRegions = new UVector(NULL, uhash_compareChars, status);\r
+                    parentRegion->containedRegions = new UVector(uprv_deleteUObject, uhash_compareUnicodeString, status);\r
                 }\r
-                parentRegion->containedRegions->addElement((void *)childRegion->id,status);\r
+\r
+                UnicodeString *childStr = new UnicodeString(status);\r
+                childStr->fastCopyFrom(childRegion->idStr);\r
+                parentRegion->containedRegions->addElement((void *)childStr,status);\r
 \r
                 // Set the parent region to be the containing region of the child.\r
                 // Regions of type GROUPING can't be set as the parent, since another region\r
@@ -281,17 +316,20 @@ void Region::loadRegionData() {
                 }\r
             }\r
         }\r
+        ures_close(mapping);\r
     }     \r
 \r
     // Create the availableRegions lists\r
-\r
-    for ( int32_t i = 0 ; i < regions->size() ; i++ ) {\r
-        Region *ar = (Region *)regions->elementAt(i);\r
-        addAvailableRegion(ar,ar->type);\r
+    int32_t pos = -1;\r
+    while ( const UHashElement* element = uhash_nextElement(regionIDMap,&pos)) {\r
+        Region *ar = (Region *)element->value.pointer;\r
+        if ( availableRegions[ar->type] == NULL ) {\r
+            availableRegions[ar->type] = new UVector(uprv_deleteUObject, uhash_compareUnicodeString, status);\r
+        }\r
+        UnicodeString *arString = new UnicodeString(ar->idStr);\r
+        availableRegions[ar->type]->addElement((void *)arString,status);\r
     }\r
 \r
-    regionDataIsLoaded = true;\r
-\r
     ures_close(territoryContainment);\r
     ures_close(worldContainment);\r
     ures_close(groupingContainment);\r
@@ -303,6 +341,12 @@ void Region::loadRegionData() {
     ures_close(rb);\r
 \r
     delete df;\r
+\r
+    ucln_i18n_registerCleanup(UCLN_I18N_REGION, region_cleanup);\r
+\r
+    regionDataIsLoaded = true;\r
+    umtx_unlock(&gRegionDataLock);\r
+\r
 }\r
 \r
 \r
@@ -321,6 +365,14 @@ Region::Region () {
         preferredValues = NULL;\r
 }\r
 \r
+Region::~Region () {\r
+        if (containedRegions) {\r
+            delete containedRegions;\r
+        }\r
+        if (preferredValues) {\r
+            delete preferredValues;\r
+        }\r
+}\r
 \r
 /**\r
  * Returns true if the two regions are equal.\r
@@ -497,18 +549,21 @@ Region::getContainedRegions( URegionType type ) const {
         const char *id = cr->next(NULL,status);\r
         const Region *r = Region::getInstance(id,status);\r
         if ( r->getType() == type ) {\r
-            result->addElement((void *)r->id,status);\r
+            result->addElement((void *)&r->idStr,status);\r
         } else {\r
             StringEnumeration *children = r->getContainedRegions(type);\r
             for ( int32_t j = 0 ; j < children->count(status) ; j++ ) {\r
                 const char *id2 = children->next(NULL,status);\r
                 const Region *r2 = Region::getInstance(id2,status);\r
-                result->addElement((void *)r2->id,status);\r
+                result->addElement((void *)&r2->idStr,status);\r
             }\r
             delete children;\r
         }\r
     }\r
-    return new RegionNameEnumeration(result,status);\r
+    delete cr;\r
+    StringEnumeration* resultEnumeration = new RegionNameEnumeration(result,status);\r
+    delete result;\r
+    return resultEnumeration;\r
 }\r
  \r
 /**\r
@@ -521,12 +576,12 @@ Region::contains(const Region &other) const {
     if (!containedRegions) {\r
           return FALSE;\r
     }\r
-    if (containedRegions->contains((void *)other.id)) {\r
+    if (containedRegions->contains((void *)&other.idStr)) {\r
         return TRUE;\r
     } else {\r
         for ( int32_t i = 0 ; i < containedRegions->size() ; i++ ) {\r
-            UErrorCode status = U_ZERO_ERROR;\r
-            const Region *cr = Region::getInstance((const char *)containedRegions->elementAt(i),status);\r
+            UnicodeString *crStr = (UnicodeString *)containedRegions->elementAt(i);\r
+            Region *cr = (Region *) uhash_get(regionIDMap,(void *)crStr);\r
             if ( cr && cr->contains(other) ) {\r
                 return TRUE;\r
             }\r
@@ -580,41 +635,21 @@ Region::getType() const {
 RegionNameEnumeration::RegionNameEnumeration(UVector *fNameList, UErrorCode& status) {\r
     pos=0;\r
     if (fNameList) {\r
-        fRegionNames = new UVector(NULL, uhash_compareChars, fNameList->size(),status);\r
+        fRegionNames = new UVector(uprv_deleteUObject, uhash_compareUnicodeString, fNameList->size(),status);\r
         for ( int32_t i = 0 ; i < fNameList->size() ; i++ ) {\r
-            char *region_name = (char *) uprv_malloc(sizeof(fNameList->elementAt(i)));\r
-            if (!region_name) {\r
-                status = U_MEMORY_ALLOCATION_ERROR;\r
-                delete fRegionNames;\r
-                fRegionNames = NULL;\r
-                return;\r
-            }\r
-            uprv_strcpy(region_name,(char *)fNameList->elementAt(i));\r
-            fRegionNames->addElement(region_name,status);\r
-            \r
+            UnicodeString* this_region_name = (UnicodeString *)fNameList->elementAt(i);\r
+            UnicodeString* new_region_name = new UnicodeString(*this_region_name);\r
+            fRegionNames->addElement((void *)new_region_name,status);          \r
         }\r
     }\r
     else { \r
-        fRegionNames = fNameList;\r
-    }\r
-}\r
-\r
-const char*\r
-RegionNameEnumeration::next(int32_t *resultLength, UErrorCode& status) {\r
-    if (U_SUCCESS(status) && pos < fRegionNames->size()) {\r
-        if (resultLength != NULL) {\r
-            *resultLength = uprv_strlen((const char *)fRegionNames->elementAt(pos));\r
-        }\r
-        return (const char *)fRegionNames->elementAt(pos++);\r
+        fRegionNames = NULL;\r
     }\r
-    return NULL;\r
 }\r
 \r
 const UnicodeString*\r
-RegionNameEnumeration::snext(UErrorCode& status) { \r
-    int32_t resultLength=0;\r
-    const char *s=next(&resultLength, status);\r
-    return setChars(s, resultLength, status);\r
+RegionNameEnumeration::snext(UErrorCode& /*status*/) { \r
+    return (const UnicodeString *)fRegionNames->elementAt(pos++);\r
 }\r
 \r
 void\r

diff --git a/icu4c/source/i18n/region_impl.h b/icu4c/source/i18n/region_impl.h
index a7e5c46082c41abda5fb869eca9daa642e74ebeb..feef9aee6554a203c5beba8f41e322029c27ac0d 100644 (file)
--- a/icu4c/source/i18n/region_impl.h
+++ b/icu4c/source/i18n/region_impl.h
@@ -28,7 +28,6 @@ public:
     virtual ~RegionNameEnumeration();\r
     static UClassID U_EXPORT2 getStaticClassID(void);\r
     virtual UClassID getDynamicClassID(void) const;\r
-    virtual const char *next(int32_t *resultLength, UErrorCode& status);\r
     virtual const UnicodeString* snext(UErrorCode& status);\r
     virtual void reset(UErrorCode& status);\r
     virtual int32_t count(UErrorCode& status) const;\r

diff --git a/icu4c/source/i18n/ucln_in.h b/icu4c/source/i18n/ucln_in.h
index 59fa80e0bbaf07c15590ec39f9347e004a1dd138..fc3565208e201b4de38b048339f12e76171440d7 100644 (file)
--- a/icu4c/source/i18n/ucln_in.h
+++ b/icu4c/source/i18n/ucln_in.h
@@ -1,7 +1,7 @@
 /*
 ******************************************************************************
 *                                                                            *
-* Copyright (C) 2001-2012, International Business Machines                   *
+* Copyright (C) 2001-2013, International Business Machines                   *
 *                Corporation and others. All Rights Reserved.                *
 *                                                                            *
 ******************************************************************************
@@ -51,6 +51,7 @@ typedef enum ECleanupI18NType {
     UCLN_I18N_INDEX_CHARACTERS,
     UCLN_I18N_GENDERINFO,
     UCLN_I18N_CDFINFO,
+    UCLN_I18N_REGION,
     UCLN_I18N_COUNT /* This must be last */
 } ECleanupI18NType;
 

diff --git a/icu4c/source/i18n/unicode/region.h b/icu4c/source/i18n/unicode/region.h
index b1333deeac66925daa3970f222f71d5cb99dcf6d..7c24f71b8e7fa14b7faf39d1da785117b35bfdf0 100644 (file)
--- a/icu4c/source/i18n/unicode/region.h
+++ b/icu4c/source/i18n/unicode/region.h
@@ -135,6 +135,13 @@ public:
      */\r
     Region();\r
 \r
+    /**\r
+     * Default Destructor.\r
+     *\r
+     * @draft ICU 51\r
+     */\r
+    ~Region();\r
+\r
 \r
     /**\r
      * Returns true if the two regions are equal.\r

diff --git a/icu4c/source/test/intltest/regiontst.cpp b/icu4c/source/test/intltest/regiontst.cpp
index 810d7c228d7fa1eb69ea909ff8fb11c89c322e28..529264bdaba82e0616d25ec2225cab7f84a42d6d 100644 (file)
--- a/icu4c/source/test/intltest/regiontst.cpp
+++ b/icu4c/source/test/intltest/regiontst.cpp
@@ -512,6 +512,7 @@ void RegionTest::TestGetContainedRegions() {
                         r->getRegionCode(),cr->getRegionCode(),containingRegion?containingRegion->getRegionCode():"NULL"); \r
                 }\r
             }\r
+            delete containedRegions;\r
         } else {\r
             errln("Known region %s was not recognized.",rd.code);\r
         }\r
@@ -538,6 +539,7 @@ void RegionTest::TestGetContainedRegionsWithType() {
                         r->getRegionCode(),cr->getRegionCode(),containingRegion?containingRegion->getRegionCode():"NULL"); \r
                 }\r
             }\r
+            delete containedRegions;\r
         } else {\r
             errln("Known region %s was not recognized.",rd.code);\r
         }\r
@@ -623,6 +625,7 @@ void RegionTest::TestGetPreferredValues() {
                     errln("Region::getPreferredValues() for region \"%s\" should have contained \"%s\" but it didn't.",r->getRegionCode(),data[i]);\r
                 }\r
             }\r
+            delete preferredValues;\r
         } else {\r
             errln("Known region %s was not recognized.",data[0]);\r
         }\r
@@ -682,6 +685,8 @@ void RegionTest::TestAvailableTerritories() {
             errln("Available territories and all territories contained in world should be the same set.\nAvailable          = %s\nContained in World = %s",\r
                 availableTerritoriesString,containedInWorldString);\r
         }\r
+        delete availableTerritories;\r
+        delete containedInWorld;\r
     }\r
 #endif /* #if !UCONFIG_NO_FORMATTING */\r
 \r