ICU-12034 Fix MeasureUnit static initialization race.

X-SVN-Rev: 38150

diff --git a/icu4j/main/classes/core/src/com/ibm/icu/util/MeasureUnit.java b/icu4j/main/classes/core/src/com/ibm/icu/util/MeasureUnit.java
index c1fcc98e8e3c01a347ad55d92f789f85e016499c..f2b2f2d3d26738f5111d1c6bafd4d5898039f825 100644 (file)
--- a/icu4j/main/classes/core/src/com/ibm/icu/util/MeasureUnit.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/util/MeasureUnit.java
@@ -1,6 +1,6 @@
 /*
  *******************************************************************************
- * Copyright (C) 2004-2015, Google Inc, International Business Machines        *
+ * Copyright (C) 2004-2016, Google Inc, International Business Machines        *
  * Corporation and others. All Rights Reserved.                                *
  *******************************************************************************
  */
@@ -40,8 +40,12 @@ public class MeasureUnit implements Serializable {
     // Used to pre-fill the cache. These same constants appear in MeasureFormat too.
     private static final String[] unitKeys = new String[]{"units", "unitsShort", "unitsNarrow"};
     
+    // Cache of MeasureUnits.
+    // All access to the cache or cacheIsPopulated flag must be synchronized on class MeasureUnit,
+    // i.e. from synchronized static methods. Beware of non-static methods.
     private static final Map<String, Map<String,MeasureUnit>> cache 
-    = new HashMap<String, Map<String,MeasureUnit>>();
+        = new HashMap<String, Map<String,MeasureUnit>>();
+    private static boolean cacheIsPopulated = false;
 
     /**
      * @internal
@@ -131,6 +135,7 @@ public class MeasureUnit implements Serializable {
      * @stable ICU 53
      */
     public synchronized static Set<String> getAvailableTypes() {
+        populateCache();            
         return Collections.unmodifiableSet(cache.keySet());
     }
 
@@ -141,6 +146,7 @@ public class MeasureUnit implements Serializable {
      * @stable ICU 53
      */
     public synchronized static Set<MeasureUnit> getAvailable(String type) {
+        populateCache();            
         Map<String, MeasureUnit> units = cache.get(type);
         // Train users not to modify returned set from the start giving us more
         // flexibility for implementation.
@@ -241,7 +247,21 @@ public class MeasureUnit implements Serializable {
         }
     };
 
-    static {
+    /**
+     * Populate the MeasureUnit cache with all types from the data.
+     * Population is done lazily, in response to MeasureUnit.getAvailable()
+     * or other API that expects to see all of the MeasureUnits.
+     *
+     * <p>At static initialization time the MeasureUnits cache is populated
+     * with public static instances (G_FORCE, METER_PER_SECOND_SQUARED, etc.) only. 
+     * Adding of others is deferred until later to avoid circular static init 
+     * dependencies with classes Currency and TimeUnit.
+     *
+     * <p>Synchronization: this function must be called from static synchronized methods only.
+     * 
+     * @internal
+     */
+    static private void populateCache() {
         // load all of the units for English, since we know that that is a superset.
         /**
          *     units{
@@ -251,6 +271,9 @@ public class MeasureUnit implements Serializable {
          *                    other{"{0} дена"}
          *                }
          */
+        if (cacheIsPopulated) {
+            return;
+        }
         ICUResourceBundle resource = (ICUResourceBundle)UResourceBundle.getBundleInstance(ICUResourceBundle.ICU_BASE_NAME, "en");
         for (String key : unitKeys) {
             try {
@@ -287,9 +310,9 @@ public class MeasureUnit implements Serializable {
         } catch (MissingResourceException e) {
             // fall through
         }
+        cacheIsPopulated = true;
     }
 
-    // Must only be called at static initialization, or inside synchronized block.
     /**
      * @internal
      * @deprecated This API is ICU internal only.

diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/MeasureUnitTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/MeasureUnitTest.java
index 19da78c023a7c3225668b2e81064e5430c95f4e2..86c34279b7e9e48e55048c0a6543a6f809de7f45 100644 (file)
--- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/MeasureUnitTest.java
+++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/MeasureUnitTest.java
@@ -1,6 +1,6 @@
 /*
  *******************************************************************************
- * Copyright (C) 2013-2015, International Business Machines Corporation and
+ * Copyright (C) 2013-2016, International Business Machines Corporation and
  * others. All Rights Reserved.
  *******************************************************************************
  */
@@ -21,6 +21,7 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Set;
 import java.util.TreeMap;
 
 import com.ibm.icu.dev.test.TestFmwk;
@@ -229,6 +230,22 @@ public class MeasureUnitTest extends TestFmwk {
      */
     public static void main(String[] args) {
         //generateConstants(); if (true) return;
+        
+        // Ticket #12034 deadlock on multi-threaded static init of MeasureUnit.
+        // The code below reliably deadlocks with ICU 56.
+        // The test is here in main() rather than in a test function so it can be made to run
+        // before anything else.
+        Thread thread = new Thread()  {
+            @Override
+            public void run() {
+                Set<String> measureUnitTypes = MeasureUnit.getAvailableTypes();
+            }
+        };
+        thread.start();
+        Currency cur = Currency.getInstance(ULocale.ENGLISH);
+        try {thread.join();} catch(InterruptedException e) {};
+        // System.out.println("Done with MeasureUnit thread test.");
+        
         new MeasureUnitTest().run(args);
     }
     
@@ -1373,6 +1390,11 @@ public class MeasureUnitTest extends TestFmwk {
         Measure twoDeg = new Measure(2, MeasureUnit.GENERIC_TEMPERATURE);
         assertEquals("2 deg temp in fr_CA", "2°", mf.format(twoDeg));
     }
+    
+    public void testPopulateCache() {
+        // Quick check that the lazily added additions to the MeasureUnit cache are present.
+        assertTrue("MeasureUnit: unexpectedly few currencies defined", MeasureUnit.getAvailable("currency").size() > 50);
+    }
 
     // DO NOT DELETE THIS FUNCTION! It may appear as dead code, but we use this to generate code
     // for MeasureFormat during the release process.