Merge pull request #1132 from Bykiev/FixBug926

Fix TempFile usage while temp dir was deleted in same process
nissl-lab · Apr 26, 2024 · 31d087c · 31d087c
2 parents 815eb29 + dedde39
commit 31d087c
Show file tree

Hide file tree

Showing 12 changed files with 169 additions and 148 deletions.
diff --git a/main/Util/TempFile.cs b/main/Util/TempFile.cs
@@ -18,31 +18,38 @@ public class TempFile
          */
         public static FileInfo CreateTempFile(String prefix, String suffix)
         {
-
-            if (dir == null)
+            if (string.IsNullOrWhiteSpace(dir))
             {
-                dir = Directory.CreateDirectory(Path.Combine(Path.GetTempPath(), "poifiles")).FullName;
+                string tempDir = Path.Combine(Path.GetTempPath(), "poifiles");
+                dir = Directory.CreateDirectory(tempDir).FullName;
             }
+
+            if (!Directory.Exists(dir))
+                Directory.CreateDirectory(dir);
+
             // Generate a unique new filename 
             string file = Path.Combine(dir, prefix + Guid.NewGuid().ToString() + suffix);
             while (File.Exists(file))
             {
                 file = Path.Combine(dir, prefix + Guid.NewGuid().ToString() + suffix);
-                Thread.Sleep(1);
             }
-            FileStream newFile = new FileStream(file, FileMode.CreateNew, FileAccess.ReadWrite);
-            newFile.Close();
+
+            using (FileStream newFile = new FileStream(file, FileMode.CreateNew, FileAccess.ReadWrite, FileShare.ReadWrite)) { };
 
             return new FileInfo(file);
         }
 
         public static string GetTempFilePath(String prefix, String suffix)
         {
-            if (dir == null)
+            if (string.IsNullOrWhiteSpace(dir))
             {
-                dir = Directory.CreateDirectory(Path.Combine(Path.GetTempPath(), "poifiles")).FullName;
+                string tempDir = Path.Combine(Path.GetTempPath(), "poifiles");
+                dir = Directory.CreateDirectory(tempDir).FullName;
             }
 
+            if (!Directory.Exists(dir))
+                Directory.CreateDirectory(dir);
+
             Random rnd = new Random(DateTime.Now.Millisecond);
             rnd.Next();
             Thread.Sleep(10);

diff --git a/ooxml/XSSF/Streaming/SheetDataWriter.cs b/ooxml/XSSF/Streaming/SheetDataWriter.cs
@@ -116,23 +116,13 @@ public void Close()
         {
             try
             {
-                _outputWriter.Flush();
-                OutputStream.Flush();
+                _outputWriter.Dispose();
+                OutputStream.Dispose();
             }
             catch
             {
 
             }
-            try
-            {
-                OutputStream.Close();
-            }
-            catch
-            {
-
-            }
-
-
         }
 
         public FileInfo TempFileInfo
@@ -554,6 +544,7 @@ public bool Dispose()
             bool ret;
             try
             {
+                _outputWriter.Close();
                 OutputStream.Close();
             }
             finally

diff --git a/testcases/main/RunSerialyAndSweepTmpFilesAttribute.cs b/testcases/main/RunSerialyAndSweepTmpFilesAttribute.cs
diff --git a/testcases/main/Util/TestTempFile.cs b/testcases/main/Util/TestTempFile.cs
@@ -0,0 +1,80 @@
+using NPOI.Util;
+using NUnit.Framework;
+using System.IO;
+using System.Threading;
+
+namespace TestCases.Util
+{
+    /// <summary>
+    /// Tests of creating temp files
+    /// </summary>
+    [TestFixture]
+    internal class TestTempFile
+    {
+        [Test]
+        public void TestCreateTempFile()
+        {
+            FileInfo fileInfo = null;
+            Assert.DoesNotThrow(() => fileInfo = TempFile.CreateTempFile("test", ".xls"));
+
+            Assert.IsTrue(fileInfo!=null && fileInfo.Exists);
+
+            string tempDirPath = Path.GetDirectoryName(fileInfo.FullName);
+
+            while(Directory.Exists(tempDirPath))
+            {
+                try
+                {
+                    Directory.Delete(tempDirPath, true);
+                }
+                catch 
+                {
+                    Thread.Sleep(5);
+                }
+            }
+
+            Assert.IsFalse(Directory.Exists(tempDirPath));
+
+            if(fileInfo!=null)
+            {
+                fileInfo.Refresh();
+                Assert.IsFalse(fileInfo.Exists);
+            }
+
+            FileInfo file = null;
+            Assert.DoesNotThrow(() => file = TempFile.CreateTempFile("test2", ".xls"));
+            Assert.IsTrue(Directory.Exists(tempDirPath));
+
+            if(file !=null && file.Exists)
+                file.Delete();
+        }
+
+        [Test]
+        public void TestGetTempFilePath()
+        {
+            string path = "";
+            Assert.DoesNotThrow(() => path = TempFile.GetTempFilePath("test", ".xls"));
+
+            Assert.IsTrue(!string.IsNullOrWhiteSpace(path));
+
+            string tempDirPath = Path.GetDirectoryName(path);
+
+            while(Directory.Exists(tempDirPath))
+            {
+                try
+                {
+                    Directory.Delete(tempDirPath, true);
+                }
+                catch 
+                {
+                    Thread.Sleep(10);
+                }
+            }
+
+            Assert.IsFalse(Directory.Exists(tempDirPath));
+
+            Assert.DoesNotThrow(() => TempFile.GetTempFilePath("test", ".xls"));
+            Assert.IsTrue(Directory.Exists(tempDirPath));
+        }
+    }
+}
diff --git a/testcases/ooxml/XSSF/Streaming/GZIPSheetDataWriterTests.cs b/testcases/ooxml/XSSF/Streaming/GZIPSheetDataWriterTests.cs
@@ -31,11 +31,10 @@ public void CleanUp()
         {
             if (_objectToTest != null)
             {
+                _objectToTest.Dispose();
+
                 if (File.Exists(_objectToTest.TemporaryFilePath()))
-                {
-                    _objectToTest.Dispose();
                     File.Delete(_objectToTest.TemporaryFilePath());
-                }
             }
         }
         [Test]

diff --git a/testcases/ooxml/XSSF/Streaming/SXSSFWorkbookTests.cs b/testcases/ooxml/XSSF/Streaming/SXSSFWorkbookTests.cs
@@ -14,15 +14,12 @@ the License.  You may obtain a copy of the License at
    See the License for the specific language governing permissions and
    limitations under the License.
 ==================================================================== */
-using System;
-using System.Collections.Generic;
-using System.IO;
-using System.Linq;
-using System.Text;
 using NPOI.SS.UserModel;
 using NPOI.XSSF.Streaming;
 using NPOI.XSSF.UserModel;
 using NUnit.Framework;
+using System;
+using System.IO;
 
 namespace TestCases.XSSF.Streaming
 {
@@ -31,6 +28,13 @@ class SXSSFWorkbookTests
     {
         private SXSSFWorkbook _objectToTest { get; set; }
 
+        [TearDown]
+        public void CleanUp()
+        {
+            if (_objectToTest != null)
+                _objectToTest.Dispose();
+        }
+
         [Test]
         public void
             CallingEmptyConstructorShouldInstanstiateNewXssfWorkbookDefaultRowAccessWindowSizeCompressTempFilesAsFalseAndUseSharedStringsTableFalse
@@ -76,6 +80,8 @@ public void IfCompressTmpFilesIsSetToTrueShouldReturnGZIPSheetDataWriter()
 
             Assert.IsTrue(result is GZIPSheetDataWriter);
 
+            if (result != null)
+                result.Close();
         }
 
         [Test]
@@ -86,6 +92,8 @@ public void IfCompressTmpFilesIsSetToFalseShouldReturnSheetDataWriter()
 
             Assert.IsTrue(result is SheetDataWriter);
 
+            if (result != null)
+                result.Close();
         }
 
         [Test]
@@ -111,7 +119,6 @@ public void IfSettingSelectedTabShouldSetSelectedTabOfXssfWorkbook()
             _objectToTest.SetSelectedTab(0);
 
             Assert.IsTrue(_objectToTest.GetSheetAt(0).IsSelected);
-
         }
 
         [Test]
@@ -124,7 +131,6 @@ public void IfSheetNameByIndexShouldGetSheetNameFromXssfWorkbook()
             _objectToTest.SetSelectedTab(0);
 
             Assert.IsTrue(_objectToTest.GetSheetAt(0).IsSelected);
-
         }
 
         [Test]
@@ -135,7 +141,6 @@ public void IfSettingSheetNameShouldChangeTheSheetNameAtTheSpecifiedIndex()
             _objectToTest.SetSheetName(0, "renamed");
 
             Assert.AreEqual("renamed", _objectToTest.GetSheetAt(0).SheetName);
-
         }
 
         [Test]
@@ -199,10 +204,8 @@ public void IfGivenTheNameOfAnExistingSheetShouldReturnTheSheet()
 
             Assert.AreEqual("1", sheet1.SheetName);
             Assert.AreEqual("2", sheet2.SheetName);
-
         }
 
-
         [Test]
         public void IfGivenTheIndexOfAnExistingSheetShouldReturnTheSheet()
         {
@@ -215,7 +218,6 @@ public void IfGivenTheIndexOfAnExistingSheetShouldReturnTheSheet()
 
             Assert.AreEqual("1", sheet1.SheetName);
             Assert.AreEqual("2", sheet2.SheetName);
-
         }
 
         [Test]
@@ -451,7 +453,6 @@ public void IfWriting20WorksheetsWith10000x100CellsUsingGzipShouldNotThrowOutOfM
             File.Delete(savePath);
         }
 
-
         private void AddCells(IWorkbook wb, int sheets, int rows, int columns, CellType type)
         {
             for (int j = 0; j < sheets; j++)
@@ -468,7 +469,6 @@ private void AddCells(IWorkbook wb, int sheets, int rows, int columns, CellType
             }
         }
 
-
         private void WriteFile(string saveAsPath, SXSSFWorkbook wb)
         {
             //Passing SXSSFWorkbook because IWorkbook does not implement .Dispose which cleans ups temporary files.

diff --git a/testcases/ooxml/XSSF/Streaming/SheetDataWriterTests.cs b/testcases/ooxml/XSSF/Streaming/SheetDataWriterTests.cs
@@ -45,11 +45,10 @@ public void CleanUp()
         {
             if (_objectToTest != null)
             {
+                _objectToTest.Dispose();
+
                 if (File.Exists(_objectToTest.TemporaryFilePath()))
-                {
-                    _objectToTest.Dispose();
                     File.Delete(_objectToTest.TemporaryFilePath());
-                }
             }
 
         }