ICU-20595 Make icu::TimeZone::AdoptDefault thread safe
correct the mutex
Remove comments about not thread safe
diff --git a/icu4c/source/i18n/timezone.cpp b/icu4c/source/i18n/timezone.cpp
index f129d8b..38e6d64 100644
--- a/icu4c/source/i18n/timezone.cpp
+++ b/icu4c/source/i18n/timezone.cpp
@@ -527,6 +527,11 @@
// -------------------------------------
+static UMutex *gDefaultZoneMutex() {
+ static UMutex* m = STATIC_NEW(UMutex);
+ return m;
+}
+
/**
* Initialize DEFAULT_ZONE from the system default time zone.
* Upon return, DEFAULT_ZONE will not be NULL, unless operator new()
@@ -536,6 +541,7 @@
{
ucln_i18n_registerCleanup(UCLN_I18N_TIMEZONE, timeZone_cleanup);
+ Mutex lock(gDefaultZoneMutex());
// If setDefault() has already been called we can skip getting the
// default zone information from the system.
if (DEFAULT_ZONE != NULL) {
@@ -557,9 +563,6 @@
TimeZone *default_zone = TimeZone::detectHostTimeZone();
- // The only way for DEFAULT_ZONE to be non-null at this point is if the user
- // made a thread-unsafe call to setDefault() or adoptDefault() in another
- // thread while this thread was doing something that required getting the default.
U_ASSERT(DEFAULT_ZONE == NULL);
DEFAULT_ZONE = default_zone;
@@ -571,7 +574,10 @@
TimeZone::createDefault()
{
umtx_initOnce(gDefaultZoneInitOnce, initDefault);
- return (DEFAULT_ZONE != NULL) ? DEFAULT_ZONE->clone() : NULL;
+ {
+ Mutex lock(gDefaultZoneMutex());
+ return (DEFAULT_ZONE != NULL) ? DEFAULT_ZONE->clone() : NULL;
+ }
}
// -------------------------------------
@@ -581,9 +587,12 @@
{
if (zone != NULL)
{
- TimeZone *old = DEFAULT_ZONE;
- DEFAULT_ZONE = zone;
- delete old;
+ {
+ Mutex lock(gDefaultZoneMutex());
+ TimeZone *old = DEFAULT_ZONE;
+ DEFAULT_ZONE = zone;
+ delete old;
+ }
ucln_i18n_registerCleanup(UCLN_I18N_TIMEZONE, timeZone_cleanup);
}
}
diff --git a/icu4c/source/i18n/unicode/timezone.h b/icu4c/source/i18n/unicode/timezone.h
index de558f9..ede0c48 100644
--- a/icu4c/source/i18n/unicode/timezone.h
+++ b/icu4c/source/i18n/unicode/timezone.h
@@ -321,10 +321,6 @@
* zone is set to the default host time zone. This call adopts the TimeZone object
* passed in; the client is no longer responsible for deleting it.
*
- * <p>This function is not thread safe. It is an error for multiple threads
- * to concurrently attempt to set the default time zone, or for any thread
- * to attempt to reference the default zone while another thread is setting it.
- *
* @param zone A pointer to the new TimeZone object to use as the default.
* @stable ICU 2.0
*/
@@ -335,8 +331,6 @@
* Same as adoptDefault(), except that the TimeZone object passed in is NOT adopted;
* the caller remains responsible for deleting it.
*
- * <p>See the thread safety note under adoptDefault().
- *
* @param zone The given timezone.
* @system
* @stable ICU 2.0
diff --git a/icu4c/source/test/intltest/tzfmttst.cpp b/icu4c/source/test/intltest/tzfmttst.cpp
index 46a640a..056ab6c 100644
--- a/icu4c/source/test/intltest/tzfmttst.cpp
+++ b/icu4c/source/test/intltest/tzfmttst.cpp
@@ -85,6 +85,7 @@
TESTCASE(5, TestFormatTZDBNames);
TESTCASE(6, TestFormatCustomZone);
TESTCASE(7, TestFormatTZDBNamesAllZoneCoverage);
+ TESTCASE(8, TestAdoptDefaultThreadSafe);
default: name = ""; break;
}
}
@@ -711,6 +712,42 @@
delete tzids;
}
+void
+TimeZoneFormatTest::TestAdoptDefaultThreadSafe(void) {
+ ThreadPool<TimeZoneFormatTest> threads(this, threadCount, &TimeZoneFormatTest::RunAdoptDefaultThreadSafeTests);
+ threads.start(); // Start all threads.
+ threads.join(); // Wait for all threads to finish.
+}
+
+static const int32_t kAdoptDefaultIteration = 10;
+static const int32_t kCreateDefaultIteration = 5000;
+static const int64_t kStartTime = 1557288964845;
+
+void TimeZoneFormatTest::RunAdoptDefaultThreadSafeTests(int32_t threadNumber) {
+ UErrorCode status = U_ZERO_ERROR;
+ if (threadNumber % 2 == 0) {
+ for (int32_t i = 0; i < kAdoptDefaultIteration; i++) {
+ std::unique_ptr<icu::StringEnumeration> timezones(
+ icu::TimeZone::createEnumeration());
+ while (const icu::UnicodeString* timezone = timezones->snext(status)) {
+ status = U_ZERO_ERROR;
+ icu::TimeZone::adoptDefault(icu::TimeZone::createTimeZone(*timezone));
+ }
+ }
+ } else {
+ int32_t rawOffset;
+ int32_t dstOffset;
+ int64_t date = kStartTime;
+ for (int32_t i = 0; i < kCreateDefaultIteration; i++) {
+ date += 6000 * i;
+ std::unique_ptr<icu::TimeZone> tz(icu::TimeZone::createDefault());
+ status = U_ZERO_ERROR;
+ tz->getOffset(date, TRUE, rawOffset, dstOffset, status);
+ status = U_ZERO_ERROR;
+ tz->getOffset(date, FALSE, rawOffset, dstOffset, status);
+ }
+ }
+}
typedef struct {
const char* text;
@@ -1301,5 +1338,4 @@
}
}
}
-
#endif /* #if !UCONFIG_NO_FORMATTING */
diff --git a/icu4c/source/test/intltest/tzfmttst.h b/icu4c/source/test/intltest/tzfmttst.h
index 89ece5e..1335a08 100644
--- a/icu4c/source/test/intltest/tzfmttst.h
+++ b/icu4c/source/test/intltest/tzfmttst.h
@@ -29,8 +29,10 @@
void TestFormatTZDBNames(void);
void TestFormatCustomZone(void);
void TestFormatTZDBNamesAllZoneCoverage(void);
+ void TestAdoptDefaultThreadSafe(void);
void RunTimeRoundTripTests(int32_t threadNumber);
+ void RunAdoptDefaultThreadSafeTests(int32_t threadNumber);
};
#endif /* #if !UCONFIG_NO_FORMATTING */
