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 162c50a..50be63e 100644
--- 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 @@
return ( algorithmic );
}
-StringEnumeration* NumberingSystem::getAvailableNames(UErrorCode &status) {
- // TODO(ticket #11908): Init-once static cache, with u_cleanup() callback.
- static StringEnumeration* availableNames = nullptr;
+namespace {
- if (U_FAILURE(status)) {
- return nullptr;
- }
+UVector* gNumsysNames = nullptr;
+UInitOnce gNumSysInitOnce = U_INITONCE_INITIALIZER;
- 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;
- }
-
- 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)) {
- 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.
- }
-
- return availableNames;
+U_CFUNC UBool U_CALLCONV numSysCleanup() {
+ delete gNumsysNames;
+ gNumsysNames = nullptr;
+ gNumSysInitOnce.reset();
+ return true;
}
-NumsysNameEnumeration::NumsysNameEnumeration(UVector *numsysNames, UErrorCode& /*status*/) {
- pos=0;
- fNumsysNames = numsysNames;
+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;
+ }
+
+ 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);
+ if (U_SUCCESS(status)) {
+ numsysNames->addElement(newElem.getAlias(), status);
+ if (U_SUCCESS(status)) {
+ newElem.orphan(); // on success, the numsysNames vector owns newElem.
+ }
+ }
+ }
+
+ ures_close(numberingSystemsInfo);
+ if (U_SUCCESS(status)) {
+ gNumsysNames = numsysNames.orphan();
+ }
+ return;
+}
+
+} // 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(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 @@
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 b798286..e76e634 100644
--- a/icu4c/source/i18n/numsys_impl.h
+++ b/icu4c/source/i18n/numsys_impl.h
@@ -26,18 +26,16 @@
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 4c13b9f..2f70a85 100644
--- a/icu4c/source/i18n/ucln_in.h
+++ b/icu4c/source/i18n/ucln_in.h
@@ -60,6 +60,7 @@
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 f1c1090..5e14ea5 100644
--- a/icu4c/source/i18n/unicode/numsys.h
+++ b/icu4c/source/i18n/unicode/numsys.h
@@ -109,6 +109,10 @@
/**
* 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 0abdd67..b4d3a58 100644
--- a/icu4c/source/test/cintltst/cnumtst.c
+++ b/icu4c/source/test/cintltst/cnumtst.c
@@ -2500,33 +2500,37 @@
}
}
- 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));
}
}
