#63291 CellFormat global cache isn't thread-safe

move date format synchronization down to where the problem instance is held.

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1856647 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/.settings/com.vaadin.designer.prefs b/.settings/com.vaadin.designer.prefs
new file mode 100644 (file)
index 0000000..83e545c
--- /dev/null
+++ b/.settings/com.vaadin.designer.prefs
@@ -0,0 +1,2 @@
+applicationTheme=Project structure is not supported.\r
+eclipse.preferences.version=1\r

diff --git a/src/java/org/apache/poi/ss/format/CellDateFormatter.java b/src/java/org/apache/poi/ss/format/CellDateFormatter.java
index b544d9ca166906ba4fae4710258a5e2f9a093778..94a4f46710df5cea31a0959b3c7b1a0858a537cd 100644 (file)
--- a/src/java/org/apache/poi/ss/format/CellDateFormatter.java
+++ b/src/java/org/apache/poi/ss/format/CellDateFormatter.java
@@ -163,7 +163,7 @@ public class CellDateFormatter extends CellFormatter {
     }
 
     /** {@inheritDoc} */
-    public void formatValue(StringBuffer toAppendTo, Object value) {
+    public synchronized void formatValue(StringBuffer toAppendTo, Object value) {
         if (value == null)
             value = 0.0;
         if (value instanceof Number) {

diff --git a/src/java/org/apache/poi/ss/format/CellFormatter.java b/src/java/org/apache/poi/ss/format/CellFormatter.java
index 2803c37e59709d3b32a49eb7ce9dfa7fdaa96740..3cb7daf1794cc0da34a27a31be7f929f0e54caf8 100644 (file)
--- a/src/java/org/apache/poi/ss/format/CellFormatter.java
+++ b/src/java/org/apache/poi/ss/format/CellFormatter.java
@@ -57,6 +57,11 @@ public abstract class CellFormatter {
 
     /**
      * Format a value according the format string.
+     * <p/>
+     * NOTE: this method must be thread safe!  In particular, if it uses a 
+     * Format instance that is not thread safe, i.e. DateFormat, this method
+     * must be synchronized, either on the method, if the format is a final 
+     * property, or on the format instance itself.
      *
      * @param toAppendTo The buffer to append to.
      * @param value      The value to format.
@@ -65,6 +70,11 @@ public abstract class CellFormatter {
 
     /**
      * Format a value according to the type, in the most basic way.
+     * <p/>
+     * NOTE: this method must be thread safe!  In particular, if it uses a 
+     * Format instance that is not thread safe, i.e. DateFormat, this method
+     * must be synchronized, either on the method, if the format is a final 
+     * property, or on the format instance itself.
      *
      * @param toAppendTo The buffer to append to.
      * @param value      The value to format.

diff --git a/src/java/org/apache/poi/ss/usermodel/DataFormatter.java b/src/java/org/apache/poi/ss/usermodel/DataFormatter.java
index d80f2bc47636a6c1f461b8ff7949e3c9bfc23091..db61f68d444697f8f44836a95530766f4ed6466b 100644 (file)
--- a/src/java/org/apache/poi/ss/usermodel/DataFormatter.java
+++ b/src/java/org/apache/poi/ss/usermodel/DataFormatter.java
@@ -309,7 +309,7 @@ public class DataFormatter implements Observer {
         return getFormat(cell.getNumericCellValue(), formatIndex, formatStr);
     }
 
-    private synchronized Format getFormat(double cellValue, int formatIndex, String formatStrIn) {
+    private Format getFormat(double cellValue, int formatIndex, String formatStrIn) {
         localeChangedObservable.checkForLocaleChange();
 
         // Might be better to separate out the n p and z formats, falling back to p when n and z are not set.

diff --git a/src/testcases/org/apache/poi/ss/usermodel/TestDataFormatter.java b/src/testcases/org/apache/poi/ss/usermodel/TestDataFormatter.java
index 96cdf9ae0da1e49e3acb96bba150528507f5ba84..014139d88fd1c06ee16ea3a1b43196e12502c655 100644 (file)
--- a/src/testcases/org/apache/poi/ss/usermodel/TestDataFormatter.java
+++ b/src/testcases/org/apache/poi/ss/usermodel/TestDataFormatter.java
@@ -939,9 +939,10 @@ public class TestDataFormatter {
 
     @Test
     public void testConcurrentCellFormat() throws Exception {
-        DataFormatter formatter = new DataFormatter();
-        doFormatTestSequential(formatter);
-        doFormatTestConcurrent(formatter);
+        DataFormatter formatter1 = new DataFormatter();
+        DataFormatter formatter2 = new DataFormatter();
+        doFormatTestSequential(formatter1);
+        doFormatTestConcurrent(formatter1, formatter2);
     }
 
     private void doFormatTestSequential(DataFormatter formatter) {
@@ -951,14 +952,21 @@ public class TestDataFormatter {
         }
     }
 
-    private void doFormatTestConcurrent(DataFormatter formatter) throws Exception {
+    private void doFormatTestConcurrent(DataFormatter formatter1, DataFormatter formatter2) throws Exception {
         ArrayList<CompletableFuture<Boolean>> futures = new ArrayList<>();
         for (int i = 0; i < 1_000; i++) {
             final int iteration = i;
             CompletableFuture<Boolean> future = CompletableFuture.supplyAsync(
                     () -> {
-                        boolean r1 = doFormatTest(formatter, 43551.50990171296, "3/27/19 12:14:15 PM", iteration);
-                        boolean r2 = doFormatTest(formatter, 36104.424780092595, "11/5/98 10:11:41 AM", iteration);
+                        boolean r1 = doFormatTest(formatter1, 43551.50990171296, "3/27/19 12:14:15 PM", iteration);
+                        boolean r2 = doFormatTest(formatter1, 36104.424780092595, "11/5/98 10:11:41 AM", iteration);
+                        return r1 && r2;
+                    });
+            futures.add(future);
+            future = CompletableFuture.supplyAsync(
+                    () -> {
+                        boolean r1 = doFormatTest(formatter2, 43551.50990171296, "3/27/19 12:14:15 PM", iteration);
+                        boolean r2 = doFormatTest(formatter2, 36104.424780092595, "11/5/98 10:11:41 AM", iteration);
                         return r1 && r2;
                     });
             futures.add(future);