From 0d22749477125691ea5c7ca8de620aa5dbdef852 Mon Sep 17 00:00:00 2001 From: Greg Woolsey Date: Sat, 30 Mar 2019 19:33:02 +0000 Subject: [PATCH] #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 --- .settings/com.vaadin.designer.prefs | 2 ++ .../poi/ss/format/CellDateFormatter.java | 2 +- .../apache/poi/ss/format/CellFormatter.java | 10 ++++++++++ .../poi/ss/usermodel/DataFormatter.java | 2 +- .../poi/ss/usermodel/TestDataFormatter.java | 20 +++++++++++++------ 5 files changed, 28 insertions(+), 8 deletions(-) create mode 100644 .settings/com.vaadin.designer.prefs diff --git a/.settings/com.vaadin.designer.prefs b/.settings/com.vaadin.designer.prefs new file mode 100644 index 0000000000..83e545c06d --- /dev/null +++ b/.settings/com.vaadin.designer.prefs @@ -0,0 +1,2 @@ +applicationTheme=Project structure is not supported. +eclipse.preferences.version=1 diff --git a/src/java/org/apache/poi/ss/format/CellDateFormatter.java b/src/java/org/apache/poi/ss/format/CellDateFormatter.java index b544d9ca16..94a4f46710 100644 --- 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 2803c37e59..3cb7daf179 100644 --- 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. + *

+ * 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. + *

+ * 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 d80f2bc476..db61f68d44 100644 --- 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 96cdf9ae0d..014139d88f 100644 --- 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> futures = new ArrayList<>(); for (int i = 0; i < 1_000; i++) { final int iteration = i; CompletableFuture 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);