From: Dominik Stadler <centic@apache.org>
Date: Mon, 26 May 2014 20:03:08 +0000 (+0000)
Subject: Fix for 56563 - Multithreading bug when reading 2 similar files
X-Git-Tag: REL_3_11_BETA1~127
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=00c69b3e15be8806716ad312ac383d2c4d272002;p=poi.git

Fix for 56563 - Multithreading bug when reading 2 similar files

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

diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFCellStyle.java b/src/java/org/apache/poi/hssf/usermodel/HSSFCellStyle.java
index 07de7baf0f..de18107683 100644
--- a/src/java/org/apache/poi/hssf/usermodel/HSSFCellStyle.java
+++ b/src/java/org/apache/poi/hssf/usermodel/HSSFCellStyle.java
@@ -101,30 +101,36 @@ public final class HSSFCellStyle implements CellStyle {
         return _format.getFormatIndex();
     }
 
+    // we keep the cached data in ThreadLocal members in order to
+    // avoid multi-threading issues when different workbooks are accessed in 
+    // multiple threads at the same time
+    private static ThreadLocal<Short> lastDateFormat = new ThreadLocal<Short>() {
+        protected Short initialValue() {
+            return Short.MIN_VALUE;
+        }
+    };
+    private static ThreadLocal<List<FormatRecord>> lastFormats = new ThreadLocal<List<FormatRecord>>();
+    private static ThreadLocal<String> getDataFormatStringCache = new ThreadLocal<String>();
+
     /**
      * Get the contents of the format string, by looking up
      *  the DataFormat against the bound workbook
      * @see org.apache.poi.hssf.usermodel.HSSFDataFormat
      * @return the format string or "General" if not found
      */
-     
-    private static short lastDateFormat = Short.MIN_VALUE;
-    private static List<FormatRecord> lastFormats = null;
-    private static String getDataFormatStringCache = null;
-    
     public String getDataFormatString() {
-        if (getDataFormatStringCache != null) {
-            if (lastDateFormat == getDataFormat() && _workbook.getFormats().equals(lastFormats)) {
-                return getDataFormatStringCache;
+        if (getDataFormatStringCache.get() != null) {
+            if (lastDateFormat.get() == getDataFormat() && _workbook.getFormats().equals(lastFormats.get())) {
+                return getDataFormatStringCache.get();
             }
         }
 
-        lastFormats = _workbook.getFormats();
-        lastDateFormat = getDataFormat();
+        lastFormats.set(_workbook.getFormats());
+        lastDateFormat.set(getDataFormat());
 
-        getDataFormatStringCache = getDataFormatString(_workbook);
+        getDataFormatStringCache.set(getDataFormatString(_workbook));
 
-        return getDataFormatStringCache;
+        return getDataFormatStringCache.get();
     }
 
     /**
@@ -862,9 +868,9 @@ public final class HSSFCellStyle implements CellStyle {
     	// Handle matching things if we cross workbooks
     	if(_workbook != source._workbook) {
 
-            lastDateFormat = Short.MIN_VALUE;
-            lastFormats = null;
-            getDataFormatStringCache = null;
+            lastDateFormat.set(Short.MIN_VALUE);
+            lastFormats.set(null);
+            getDataFormatStringCache.set(null);
     	   
 			// Then we need to clone the format string,
 			//  and update the format record for this
diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestCellStyle.java b/src/testcases/org/apache/poi/hssf/usermodel/TestCellStyle.java
index e57191b646..bd874629f8 100644
--- a/src/testcases/org/apache/poi/hssf/usermodel/TestCellStyle.java
+++ b/src/testcases/org/apache/poi/hssf/usermodel/TestCellStyle.java
@@ -26,7 +26,11 @@ import java.util.Date;
 import junit.framework.TestCase;
 
 import org.apache.poi.hssf.HSSFTestDataSamples;
+import org.apache.poi.ss.usermodel.Cell;
 import org.apache.poi.ss.usermodel.CellStyle;
+import org.apache.poi.ss.usermodel.Row;
+import org.apache.poi.ss.usermodel.Sheet;
+import org.apache.poi.ss.usermodel.Workbook;
 import org.apache.poi.util.TempFile;
 
 /**
@@ -405,4 +409,62 @@ public final class TestCellStyle extends TestCase {
     	assertEquals(false, r.getCell(0).getCellStyle().getShrinkToFit());
     	assertEquals(true,  r.getCell(1).getCellStyle().getShrinkToFit());
     }
+    
+    
+    
+    private static class CellFormatBugExample extends Thread {
+        private String fileName;
+        private Throwable exception = null;
+
+        public CellFormatBugExample(String fileName) {
+            this.fileName = fileName;
+        }
+
+        @Override
+        public void run() {
+            try {
+                for(int i = 0;i< 10;i++) {
+                    Workbook wb = HSSFTestDataSamples.openSampleWorkbook(fileName);
+                    Sheet sheet = wb.getSheetAt(0);
+        
+                    for (Row row : sheet) {
+                        for (Integer idxCell = 0; idxCell < row.getLastCellNum(); idxCell++) {
+        
+                            Cell cell = row.getCell(idxCell);
+                            cell.getCellStyle().getDataFormatString();
+                            if (cell.getCellType() == HSSFCell.CELL_TYPE_NUMERIC) {
+                                boolean isDate = HSSFDateUtil.isCellDateFormatted(cell);
+                                if (idxCell > 0 && isDate) {
+                                    fail("cell " + idxCell + " is not a date: " + idxCell.toString());
+                                }
+                            }
+                        }
+                    }
+                }
+            } catch (Throwable e) {
+                exception = e;
+            }
+        }
+
+        public Throwable getException() {
+            return exception;
+        }
+    };
+    
+    public void test56563() throws Throwable {
+        CellFormatBugExample threadA = new CellFormatBugExample("56563a.xls");
+        threadA.start();
+        CellFormatBugExample threadB = new CellFormatBugExample("56563b.xls");
+        threadB.start();
+        
+        threadA.join();
+        threadB.join();
+        
+        if(threadA.getException() != null) {
+            throw threadA.getException();
+        }
+        if(threadB.getException() != null) {
+            throw threadB.getException();
+        }
+    }
 }
diff --git a/test-data/spreadsheet/56563a.xls b/test-data/spreadsheet/56563a.xls
new file mode 100644
index 0000000000..7fb4aa4d53
Binary files /dev/null and b/test-data/spreadsheet/56563a.xls differ
diff --git a/test-data/spreadsheet/56563b.xls b/test-data/spreadsheet/56563b.xls
new file mode 100644
index 0000000000..7fb4aa4d53
Binary files /dev/null and b/test-data/spreadsheet/56563b.xls differ