#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
This commit is contained in:
Greg Woolsey 2019-03-30 19:33:02 +00:00
parent 30851e44a6
commit 0d22749477
5 changed files with 28 additions and 8 deletions

View File

@ -0,0 +1,2 @@
applicationTheme=Project structure is not supported.
eclipse.preferences.version=1

View File

@ -163,7 +163,7 @@ public class CellDateFormatter extends CellFormatter {
} }
/** {@inheritDoc} */ /** {@inheritDoc} */
public void formatValue(StringBuffer toAppendTo, Object value) { public synchronized void formatValue(StringBuffer toAppendTo, Object value) {
if (value == null) if (value == null)
value = 0.0; value = 0.0;
if (value instanceof Number) { if (value instanceof Number) {

View File

@ -57,6 +57,11 @@ public abstract class CellFormatter {
/** /**
* Format a value according the format string. * 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 toAppendTo The buffer to append to.
* @param value The value to format. * @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. * 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 toAppendTo The buffer to append to.
* @param value The value to format. * @param value The value to format.

View File

@ -309,7 +309,7 @@ public class DataFormatter implements Observer {
return getFormat(cell.getNumericCellValue(), formatIndex, formatStr); 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(); 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. // Might be better to separate out the n p and z formats, falling back to p when n and z are not set.

View File

@ -939,9 +939,10 @@ public class TestDataFormatter {
@Test @Test
public void testConcurrentCellFormat() throws Exception { public void testConcurrentCellFormat() throws Exception {
DataFormatter formatter = new DataFormatter(); DataFormatter formatter1 = new DataFormatter();
doFormatTestSequential(formatter); DataFormatter formatter2 = new DataFormatter();
doFormatTestConcurrent(formatter); doFormatTestSequential(formatter1);
doFormatTestConcurrent(formatter1, formatter2);
} }
private void doFormatTestSequential(DataFormatter formatter) { 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<>(); ArrayList<CompletableFuture<Boolean>> futures = new ArrayList<>();
for (int i = 0; i < 1_000; i++) { for (int i = 0; i < 1_000; i++) {
final int iteration = i; final int iteration = i;
CompletableFuture<Boolean> future = CompletableFuture.supplyAsync( CompletableFuture<Boolean> future = CompletableFuture.supplyAsync(
() -> { () -> {
boolean r1 = doFormatTest(formatter, 43551.50990171296, "3/27/19 12:14:15 PM", iteration); boolean r1 = doFormatTest(formatter1, 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 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; return r1 && r2;
}); });
futures.add(future); futures.add(future);