From: Shane Carr <shane@unicode.org>
Date: Thu, 1 Mar 2018 00:58:47 +0000 (+0000)
Subject: ICU-13606 Fixing race condition in MeasureFormat.
X-Git-Tag: release-61-rc~31
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=33709da06abd3b88c591fb38c67bfabd54bde76b;p=icu

ICU-13606 Fixing race condition in MeasureFormat.

X-SVN-Rev: 41025
---

diff --git a/icu4c/source/i18n/measfmt.cpp b/icu4c/source/i18n/measfmt.cpp
index 0d9d2f8b8e0..5970262de1a 100644
--- a/icu4c/source/i18n/measfmt.cpp
+++ b/icu4c/source/i18n/measfmt.cpp
@@ -1062,9 +1062,13 @@ UnicodeString &MeasureFormat::formatNumeric(
     }
 
     // Format time. draft becomes something like '5:30:45'
+    // #13606: DateFormat is not thread-safe, but MeasureFormat advertises itself as thread-safe.
     FieldPosition smallestFieldPosition(smallestField);
     UnicodeString draft;
+    static UMutex dateFmtMutex = U_MUTEX_INITIALIZER;
+    umtx_lock(&dateFmtMutex);
     dateFmt.format(date, draft, smallestFieldPosition, status);
+    umtx_unlock(&dateFmtMutex);
 
     // If we find field for smallest amount replace it with the formatted
     // smallest amount from above taking care to replace the integer part
diff --git a/icu4j/main/classes/core/src/com/ibm/icu/text/MeasureFormat.java b/icu4j/main/classes/core/src/com/ibm/icu/text/MeasureFormat.java
index 500ee5dc3b7..9ae8eb69f78 100644
--- a/icu4j/main/classes/core/src/com/ibm/icu/text/MeasureFormat.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/text/MeasureFormat.java
@@ -933,10 +933,15 @@ public class MeasureFormat extends UFormat {
         if (intFieldPosition.getBeginIndex() == 0 && intFieldPosition.getEndIndex() == 0) {
             throw new IllegalStateException();
         }
+
         // Format our duration as a date, but keep track of where the smallest field is
         // so that we can use it to replace the integer portion of the smallest value.
+        // #13606: DateFormat is not thread-safe, but MeasureFormat advertises itself as thread-safe.
         FieldPosition smallestFieldPosition = new FieldPosition(smallestField);
-        String draft = formatter.format(duration, new StringBuffer(), smallestFieldPosition).toString();
+        String draft;
+        synchronized (formatter) {
+            draft = formatter.format(duration, new StringBuffer(), smallestFieldPosition).toString();
+        }
 
         try {
             // If we find the smallest field
diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/MeasureUnitThreadTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/MeasureUnitThreadTest.java
index e86ed12fe3a..05fb18f03b2 100644
--- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/MeasureUnitThreadTest.java
+++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/MeasureUnitThreadTest.java
@@ -8,7 +8,10 @@ import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
 import com.ibm.icu.dev.test.TestFmwk;
+import com.ibm.icu.impl.DontCareFieldPosition;
+import com.ibm.icu.text.MeasureFormat;
 import com.ibm.icu.util.Currency;
+import com.ibm.icu.util.Measure;
 import com.ibm.icu.util.MeasureUnit;
 import com.ibm.icu.util.ULocale;
 
@@ -30,5 +33,61 @@ public class MeasureUnitThreadTest extends TestFmwk {
         Currency.getInstance(ULocale.ENGLISH);
         try {thread.join();} catch(InterruptedException e) {};
     }
+
+    static class NumericMeasureThread extends Thread {
+        final MeasureFormat mf;
+        final Measure[] arg;
+        final String expected;
+        volatile boolean running = true;
+        AssertionError error;
+
+        NumericMeasureThread(Measure[] arg, String expected) {
+            this.mf = MeasureFormat.getInstance(ULocale.ENGLISH, MeasureFormat.FormatWidth.NUMERIC);
+            this.arg = arg;
+            this.expected = expected;
+            this.error = null;
+        }
+
+        @Override
+        public void run() {
+            while (running) {
+                try {
+                    StringBuilder sb = new StringBuilder();
+                    mf.formatMeasures(sb, DontCareFieldPosition.INSTANCE, arg);
+                    org.junit.Assert.assertEquals(expected, sb.toString());
+                } catch (AssertionError e) {
+                    error = e;
+                    break;
+                }
+            }
+        }
+    }
+
+    // Race in formatMeasures with width NUMERIC:
+    // http://bugs.icu-project.org/trac/ticket/13606
+    @Test
+    public void NumericRaceTest() throws InterruptedException {
+        NumericMeasureThread t1 = new NumericMeasureThread(new Measure[] {
+          new Measure(3, MeasureUnit.MINUTE),
+          new Measure(4, MeasureUnit.SECOND)
+        }, "3:04");
+        NumericMeasureThread t2 = new NumericMeasureThread(new Measure[] {
+          new Measure(5, MeasureUnit.MINUTE),
+          new Measure(6, MeasureUnit.SECOND)
+        }, "5:06");
+        t1.start();
+        t2.start();
+        Thread.sleep(5);
+        t1.running = false;
+        t2.running = false;
+        t1.join();
+        t2.join();
+        if (t1.error != null) {
+            throw new AssertionError("Failure in thread 1", t1.error);
+        }
+        if (t2.error != null) {
+            throw new AssertionError("Failure in thread 2", t2.error);
+        }
+    }
 }