ICU-11908 NumberingSystem, fix the memory management of static cache of numsys names.

Add thread safe cache initialization.

diff --git a/icu4c/source/i18n/numsys.cpp b/icu4c/source/i18n/numsys.cpp
index 162c50a5fe96b54c779daae3598679f7c646ead7..50be63e7b93c6625431f754ed92db0a763ff2e23 100644 (file)
--- a/icu4c/source/i18n/numsys.cpp
+++ b/icu4c/source/i18n/numsys.cpp
@@ -26,6 +26,8 @@
 #include "unicode/numsys.h"
 #include "cstring.h"
 #include "uassert.h"
+#include "ucln_in.h"
+#include "umutex.h"
 #include "uresimp.h"
 #include "numsys_impl.h"
 
@@ -266,75 +268,82 @@ UBool NumberingSystem::isAlgorithmic() const {
     return ( algorithmic );
 }
 
-StringEnumeration* NumberingSystem::getAvailableNames(UErrorCode &status) {
-    // TODO(ticket #11908): Init-once static cache, with u_cleanup() callback.
-    static StringEnumeration* availableNames = nullptr;
+namespace {
+
+UVector* gNumsysNames = nullptr;
+UInitOnce gNumSysInitOnce = U_INITONCE_INITIALIZER;
+
+U_CFUNC UBool U_CALLCONV numSysCleanup() {
+    delete gNumsysNames;
+    gNumsysNames = nullptr;
+    gNumSysInitOnce.reset();
+    return true;
+}
 
+U_CFUNC void initNumsysNames(UErrorCode &status) {
+    U_ASSERT(gNumsysNames == nullptr);
+    ucln_i18n_registerCleanup(UCLN_I18N_NUMSYS, numSysCleanup);
+
+    // TODO: Simple array of UnicodeString objects, based on length of table resource?
+    LocalPointer<UVector> numsysNames(new UVector(uprv_deleteUObject, nullptr, status), status);
     if (U_FAILURE(status)) {
-        return nullptr;
+        return;
     }
 
-    if ( availableNames == nullptr ) {
-        // TODO: Simple array of UnicodeString objects, based on length of table resource?
-        LocalPointer<UVector> numsysNames(new UVector(uprv_deleteUObject, nullptr, status), status);
-        if (U_FAILURE(status)) {
-            return nullptr;
-        }
-        
-        UErrorCode rbstatus = U_ZERO_ERROR;
-        UResourceBundle *numberingSystemsInfo = ures_openDirect(nullptr, "numberingSystems", &rbstatus);
-        numberingSystemsInfo = ures_getByKey(numberingSystemsInfo, "numberingSystems", numberingSystemsInfo, &rbstatus);
-        if (U_FAILURE(rbstatus)) {
-            // Don't stomp on the catastrophic failure of OOM.
-            if (rbstatus == U_MEMORY_ALLOCATION_ERROR) {
-                status = rbstatus;
-            } else {
-                status = U_MISSING_RESOURCE_ERROR;
-            }
-            ures_close(numberingSystemsInfo);
-            return nullptr;
+    UErrorCode rbstatus = U_ZERO_ERROR;
+    UResourceBundle *numberingSystemsInfo = ures_openDirect(nullptr, "numberingSystems", &rbstatus);
+    numberingSystemsInfo =
+            ures_getByKey(numberingSystemsInfo, "numberingSystems", numberingSystemsInfo, &rbstatus);
+    if (U_FAILURE(rbstatus)) {
+        // Don't stomp on the catastrophic failure of OOM.
+        if (rbstatus == U_MEMORY_ALLOCATION_ERROR) {
+            status = rbstatus;
+        } else {
+            status = U_MISSING_RESOURCE_ERROR;
         }
+        ures_close(numberingSystemsInfo);
+        return;
+    }
 
-        while ( ures_hasNext(numberingSystemsInfo) && U_SUCCESS(status) ) {
-            LocalUResourceBundlePointer nsCurrent(ures_getNextResource(numberingSystemsInfo, nullptr, &rbstatus));
-            if (rbstatus == U_MEMORY_ALLOCATION_ERROR) {
-                status = rbstatus; // we want to report OOM failure back to the caller.
-                break;
-            }
-            const char *nsName = ures_getKey(nsCurrent.getAlias());
-            LocalPointer<UnicodeString> newElem(new UnicodeString(nsName, -1, US_INV), status);
+    while ( ures_hasNext(numberingSystemsInfo) && U_SUCCESS(status) ) {
+        LocalUResourceBundlePointer nsCurrent(ures_getNextResource(numberingSystemsInfo, nullptr, &rbstatus));
+        if (rbstatus == U_MEMORY_ALLOCATION_ERROR) {
+            status = rbstatus; // we want to report OOM failure back to the caller.
+            break;
+        }
+        const char *nsName = ures_getKey(nsCurrent.getAlias());
+        LocalPointer<UnicodeString> newElem(new UnicodeString(nsName, -1, US_INV), status);
+        if (U_SUCCESS(status)) {
+            numsysNames->addElement(newElem.getAlias(), status);
             if (U_SUCCESS(status)) {
-                numsysNames->addElement(newElem.getAlias(), status);
-                if (U_SUCCESS(status)) {
-                    newElem.orphan(); // on success, the numsysNames vector owns newElem.
-                }
+                newElem.orphan(); // on success, the numsysNames vector owns newElem.
             }
         }
+    }
 
-        ures_close(numberingSystemsInfo);
-        if (U_FAILURE(status)) {
-            return nullptr;
-        }
-        availableNames = new NumsysNameEnumeration(numsysNames.getAlias(), status);
-        if (availableNames == nullptr) {
-            status = U_MEMORY_ALLOCATION_ERROR;
-            return nullptr;
-        }
-        numsysNames.orphan();  // The names got adopted.
+    ures_close(numberingSystemsInfo);
+    if (U_SUCCESS(status)) {
+        gNumsysNames = numsysNames.orphan();
     }
+    return;
+}
 
-    return availableNames;
+}   // end anonymous namespace
+
+StringEnumeration* NumberingSystem::getAvailableNames(UErrorCode &status) {
+    umtx_initOnce(gNumSysInitOnce, &initNumsysNames, status);
+    LocalPointer<StringEnumeration> result(new NumsysNameEnumeration(status), status);
+    return result.orphan();
 }
 
-NumsysNameEnumeration::NumsysNameEnumeration(UVector *numsysNames, UErrorCode& /*status*/) {
-    pos=0;
-    fNumsysNames = numsysNames;
+NumsysNameEnumeration::NumsysNameEnumeration(UErrorCode& status) : pos(0) {
+    (void)status;
 }
 
 const UnicodeString*
 NumsysNameEnumeration::snext(UErrorCode& status) {
-    if (U_SUCCESS(status) && (fNumsysNames != nullptr) && (pos < fNumsysNames->size())) {
-        return (const UnicodeString*)fNumsysNames->elementAt(pos++);
+    if (U_SUCCESS(status) && (gNumsysNames != nullptr) && (pos < gNumsysNames->size())) {
+        return (const UnicodeString*)gNumsysNames->elementAt(pos++);
     }
     return nullptr;
 }
@@ -346,11 +355,10 @@ NumsysNameEnumeration::reset(UErrorCode& /*status*/) {
 
 int32_t
 NumsysNameEnumeration::count(UErrorCode& /*status*/) const {
-    return (fNumsysNames==nullptr) ? 0 : fNumsysNames->size();
+    return (gNumsysNames==nullptr) ? 0 : gNumsysNames->size();
 }
 
 NumsysNameEnumeration::~NumsysNameEnumeration() {
-    delete fNumsysNames;
 }
 U_NAMESPACE_END
 

diff --git a/icu4c/source/i18n/numsys_impl.h b/icu4c/source/i18n/numsys_impl.h
index b798286bf1d417dbb68eb6abd0ffe96b2fa6ab55..e76e634dd23c072e3e3791a3c82af5923f987c2e 100644 (file)
--- a/icu4c/source/i18n/numsys_impl.h
+++ b/icu4c/source/i18n/numsys_impl.h
@@ -26,18 +26,16 @@ U_NAMESPACE_BEGIN
 
 class NumsysNameEnumeration : public StringEnumeration {
 public:
-    // NumsysNameEnumeration instance adopts numsysNames
-    NumsysNameEnumeration(UVector *numsysNames, UErrorCode& status);
+    NumsysNameEnumeration(UErrorCode& status);
 
     virtual ~NumsysNameEnumeration();
     static UClassID U_EXPORT2 getStaticClassID(void);
-    virtual UClassID getDynamicClassID(void) const;
-    virtual const UnicodeString* snext(UErrorCode& status);
-    virtual void reset(UErrorCode& status);
-    virtual int32_t count(UErrorCode& status) const;
+    virtual UClassID getDynamicClassID(void) const override;
+    virtual const UnicodeString* snext(UErrorCode& status) override;
+    virtual void reset(UErrorCode& status) override;
+    virtual int32_t count(UErrorCode& status) const override;
 private:
     int32_t pos;
-    UVector *fNumsysNames = nullptr;
 };
 
 U_NAMESPACE_END

diff --git a/icu4c/source/i18n/ucln_in.h b/icu4c/source/i18n/ucln_in.h
index 4c13b9ffcb539a32829572b22babf030409e5557..2f70a8500e1c09eaff3011342c390b799fa76c87 100644 (file)
--- a/icu4c/source/i18n/ucln_in.h
+++ b/icu4c/source/i18n/ucln_in.h
@@ -60,6 +60,7 @@ typedef enum ECleanupI18NType {
     UCLN_I18N_CDFINFO,
     UCLN_I18N_REGION,
     UCLN_I18N_LIST_FORMATTER,
+    UCLN_I18N_NUMSYS,
     UCLN_I18N_COUNT /* This must be last */
 } ECleanupI18NType;
 

diff --git a/icu4c/source/i18n/unicode/numsys.h b/icu4c/source/i18n/unicode/numsys.h
index f1c1090279db089b98ae9f8e81f2448e898f4cde..5e14ea5c3111eff89c3bdf192d519848844faf16 100644 (file)
--- a/icu4c/source/i18n/unicode/numsys.h
+++ b/icu4c/source/i18n/unicode/numsys.h
@@ -109,6 +109,10 @@ public:
     /**
      * Return a StringEnumeration over all the names of numbering systems known to ICU.
      * The numbering system names will be in alphabetical (invariant) order.
+     *
+     * The returned StringEnumeration is owned by the caller, who must delete it when
+     * finished with it.
+     *
      * @stable ICU 4.2
      */
      static StringEnumeration * U_EXPORT2 getAvailableNames(UErrorCode& status);

diff --git a/icu4c/source/test/cintltst/cnumtst.c b/icu4c/source/test/cintltst/cnumtst.c
index 0abdd67423f28ac4a2f42d748347e43f0e04f3c2..b4d3a581399795e6262f1048d913a32626121c09 100644 (file)
--- a/icu4c/source/test/cintltst/cnumtst.c
+++ b/icu4c/source/test/cintltst/cnumtst.c
@@ -2500,33 +2500,37 @@ static void TestUNumberingSystem(void) {
         }
     }
 
-    status = U_ZERO_ERROR;
-    uenum = unumsys_openAvailableNames(&status);
-    if ( U_SUCCESS(status) ) {
-        int32_t numsysCount = 0;
-        // sanity check for a couple of number systems that must be in the enumeration
-        UBool foundLatn = FALSE;
-        UBool foundArab = FALSE;
-        while ( (numsys = uenum_next(uenum, NULL, &status)) != NULL && U_SUCCESS(status) ) {
-            status = U_ZERO_ERROR;
-            unumsys = unumsys_openByName(numsys, &status);
-            if ( U_SUCCESS(status) ) {
-                numsysCount++;
-                if ( uprv_strcmp(numsys, "latn") ) foundLatn = TRUE;
-                if ( uprv_strcmp(numsys, "arab") ) foundArab = TRUE;
-                unumsys_close(unumsys);
-            } else {
-                log_err("unumsys_openAvailableNames includes %s but unumsys_openByName on it fails with status %s\n",
-                        numsys, myErrorName(status));
+    for (int i=0; i<3; ++i) {
+        // Run the test of unumsys_openAvailableNames() multiple times.
+        // Helps verify the management of the internal cache of the names.
+        status = U_ZERO_ERROR;
+        uenum = unumsys_openAvailableNames(&status);
+        if ( U_SUCCESS(status) ) {
+            int32_t numsysCount = 0;
+            // sanity check for a couple of number systems that must be in the enumeration
+            UBool foundLatn = FALSE;
+            UBool foundArab = FALSE;
+            while ( (numsys = uenum_next(uenum, NULL, &status)) != NULL && U_SUCCESS(status) ) {
+                status = U_ZERO_ERROR;
+                unumsys = unumsys_openByName(numsys, &status);
+                if ( U_SUCCESS(status) ) {
+                    numsysCount++;
+                    if ( uprv_strcmp(numsys, "latn") ) foundLatn = TRUE;
+                    if ( uprv_strcmp(numsys, "arab") ) foundArab = TRUE;
+                    unumsys_close(unumsys);
+                } else {
+                    log_err("unumsys_openAvailableNames includes %s but unumsys_openByName on it fails with status %s\n",
+                            numsys, myErrorName(status));
+                }
             }
+            uenum_close(uenum);
+            if ( numsysCount < 40 || !foundLatn || !foundArab ) {
+                log_err("unumsys_openAvailableNames results incomplete: numsysCount %d, foundLatn %d, foundArab %d\n",
+                        numsysCount, foundLatn, foundArab);
+            }
+        } else {
+            log_data_err("unumsys_openAvailableNames fails with status %s\n", myErrorName(status));
         }
-        uenum_close(uenum);
-        if ( numsysCount < 40 || !foundLatn || !foundArab ) {
-            log_err("unumsys_openAvailableNames results incomplete: numsysCount %d, foundLatn %d, foundArab %d\n",
-                    numsysCount, foundLatn, foundArab);
-        }
-    } else {
-        log_data_err("unumsys_openAvailableNames fails with status %s\n", myErrorName(status));
     }
 }