ICU-20595 Make icu::TimeZone::AdoptDefault thread safe

correct the mutex

Remove comments about not thread safe

icu4c/source/i18n/timezone.cpp

@@ -527,6 +527,11 @@ TimeZone::detectHostTimeZone()
 
 // -------------------------------------
 
+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 @@ static void U_CALLCONV initDefault()
 {
     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 @@ static void U_CALLCONV initDefault()
 
     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* U_EXPORT2
 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 @@ TimeZone::adoptDefault(TimeZone* zone)
 {
     if (zone != NULL)
     {
-        TimeZone *old = DEFAULT_ZONE;
+        {
-        DEFAULT_ZONE = zone;
+            Mutex lock(gDefaultZoneMutex());
-        delete old;
+            TimeZone *old = DEFAULT_ZONE;
+            DEFAULT_ZONE = zone;
+            delete old;
+        }
         ucln_i18n_registerCleanup(UCLN_I18N_TIMEZONE, timeZone_cleanup);
     }
 }

icu4c/source/i18n/unicode/timezone.h

@@ -321,10 +321,6 @@ public:
      * 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 @@ public:
      * 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

icu4c/source/test/intltest/tzfmttst.cpp

@@ -85,6 +85,7 @@ TimeZoneFormatTest::runIndexedTest( int32_t index, UBool exec, const char* &name
         TESTCASE(5, TestFormatTZDBNames);
         TESTCASE(6, TestFormatCustomZone);
         TESTCASE(7, TestFormatTZDBNamesAllZoneCoverage);
+        TESTCASE(8, TestAdoptDefaultThreadSafe);
     default: name = ""; break;
     }
 }
@@ -711,6 +712,42 @@ void TimeZoneFormatTest::RunTimeRoundTripTests(int32_t threadNumber) {
     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 @@ TimeZoneFormatTest::TestFormatTZDBNamesAllZoneCoverage(void) {
         }
     }
 }
-
 #endif /* #if !UCONFIG_NO_FORMATTING */

icu4c/source/test/intltest/tzfmttst.h

@@ -29,8 +29,10 @@ class TimeZoneFormatTest : public IntlTest {
     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 */