diff options
4 files changed, 35 insertions, 41 deletions
diff --git a/poi-examples/src/main/java/org/apache/poi/examples/util/TempFileUtils.java b/poi-examples/src/main/java/org/apache/poi/examples/util/TempFileUtils.java index 7d2d01b409..ac6fa02e58 100644 --- a/poi-examples/src/main/java/org/apache/poi/examples/util/TempFileUtils.java +++ b/poi-examples/src/main/java/org/apache/poi/examples/util/TempFileUtils.java @@ -19,18 +19,19 @@ package org.apache.poi.examples.util; -import java.io.File; - +import org.apache.poi.util.DefaultTempFileCreationStrategy; import org.apache.poi.util.TempFile; +import java.io.File; +import java.nio.file.Paths; + public final class TempFileUtils { private TempFileUtils() { } @SuppressWarnings("java:S106") public static void checkTempFiles() { - String tmpDir = System.getProperty(TempFile.JAVA_IO_TMPDIR) + "/poifiles"; - File tempDir = new File(tmpDir); + File tempDir = Paths.get(System.getProperty(TempFile.JAVA_IO_TMPDIR), DefaultTempFileCreationStrategy.POIFILES).toFile(); if(tempDir.exists()) { String[] tempFiles = tempDir.list(); if(tempFiles != null && tempFiles.length > 0) { diff --git a/poi-ooxml/src/test/java/org/apache/poi/util/tests/TestTempFileThreaded.java b/poi-ooxml/src/test/java/org/apache/poi/util/tests/TestTempFileThreaded.java index 678280e7b1..625fa448e4 100644 --- a/poi-ooxml/src/test/java/org/apache/poi/util/tests/TestTempFileThreaded.java +++ b/poi-ooxml/src/test/java/org/apache/poi/util/tests/TestTempFileThreaded.java @@ -24,6 +24,7 @@ import java.io.File; import java.io.FileOutputStream; import java.io.IOException; import java.io.OutputStream; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.HashMap; import java.util.LinkedList; @@ -76,10 +77,11 @@ class TestTempFileThreaded { public static void setUpClass() throws IOException { String tmpDir = System.getProperty(JAVA_IO_TMPDIR); if (tmpDir == null) { - throw new IOException("Systems temporary directory not defined - set the -D" + JAVA_IO_TMPDIR + " jvm property!"); + throw new IOException("System's temporary directory not defined - set the -D" + JAVA_IO_TMPDIR + " jvm property!"); } - TempFile.setTempFileCreationStrategy(createTempFileCreationStrategy(new File(new File(tmpDir, POIFILES), "TestTempFileThreaded"))); + TempFile.setTempFileCreationStrategy(createTempFileCreationStrategy( + Paths.get(tmpDir, POIFILES, "TestTempFileThreaded").toFile())); } @BeforeEach diff --git a/poi/src/main/java/org/apache/poi/util/DefaultTempFileCreationStrategy.java b/poi/src/main/java/org/apache/poi/util/DefaultTempFileCreationStrategy.java index 656ff73773..b4105583a2 100644 --- a/poi/src/main/java/org/apache/poi/util/DefaultTempFileCreationStrategy.java +++ b/poi/src/main/java/org/apache/poi/util/DefaultTempFileCreationStrategy.java @@ -23,7 +23,10 @@ import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; import java.nio.file.attribute.FileAttribute; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; /** * Default implementation of the {@link TempFileCreationStrategy} used by {@link TempFile}: @@ -38,13 +41,17 @@ import java.nio.file.attribute.FileAttribute; * processes or limited temporary storage. */ public class DefaultTempFileCreationStrategy implements TempFileCreationStrategy { + /** Name of POI files directory in temporary directory. */ public static final String POIFILES = "poifiles"; /** To use files.deleteOnExit after clean JVM exit, set the <code>-Dpoi.delete.tmp.files.on.exit</code> JVM property */ public static final String DELETE_FILES_ON_EXIT = "poi.delete.tmp.files.on.exit"; /** The directory where the temporary files will be created (<code>null</code> to use the default directory). */ - private File dir; + private volatile File dir; + + /** The lock to make dir initialized only once. */ + private final Lock dirLock = new ReentrantLock(); /** * Creates the strategy so that it creates the temporary files in the default directory. @@ -67,36 +74,22 @@ public class DefaultTempFileCreationStrategy implements TempFileCreationStrategy } private void createPOIFilesDirectory() throws IOException { - // Identify and create our temp dir, if needed + // Create our temp dir only once by double-checked locking // The directory is not deleted, even if it was created by this TempFileCreationStrategy if (dir == null) { - String tmpDir = System.getProperty(JAVA_IO_TMPDIR); - if (tmpDir == null) { - throw new IOException("Systems temporary directory not defined - set the -D"+JAVA_IO_TMPDIR+" jvm property!"); + dirLock.lock(); + try { + if (dir == null) { + String tmpDir = System.getProperty(JAVA_IO_TMPDIR); + if (tmpDir == null) { + throw new IOException("System's temporary directory not defined - set the -D" + JAVA_IO_TMPDIR + " jvm property!"); + } + Path dirPath = Paths.get(tmpDir, POIFILES); + dir = Files.createDirectories(dirPath).toFile(); + } + } finally { + dirLock.unlock(); } - dir = new File(tmpDir, POIFILES); - } - - createTempDirectory(dir); - } - - /** - * Attempt to create a directory, including any necessary parent directories. - * Does nothing if directory already exists. - * The method is synchronized to ensure that multiple threads don't try to create the directory at the same time. - * - * @param directory the directory to create - * @throws IOException if unable to create temporary directory or it is not a directory - */ - private synchronized void createTempDirectory(File directory) throws IOException { - // create directory if it doesn't exist - final boolean dirExists = (directory.exists() || directory.mkdirs()); - - if (!dirExists) { - throw new IOException("Could not create temporary directory '" + directory + "'"); - } - else if (!directory.isDirectory()) { - throw new IOException("Could not create temporary directory. '" + directory + "' exists but is not a directory."); } } @@ -124,10 +117,7 @@ public class DefaultTempFileCreationStrategy implements TempFileCreationStrategy createPOIFilesDirectory(); // Generate a unique new filename - // FIXME: Java 7+: use java.nio.file.Files#createTempDirectory - final long n = RandomSingleton.getInstance().nextLong(); - File newDirectory = new File(dir, prefix + Long.toString(n)); - createTempDirectory(newDirectory); + File newDirectory = Files.createTempDirectory(dir.toPath(), prefix).toFile(); //this method appears to be only used in tests, so it is probably ok to use deleteOnExit newDirectory.deleteOnExit(); diff --git a/poi/src/main/java/org/apache/poi/util/TempFile.java b/poi/src/main/java/org/apache/poi/util/TempFile.java index e0a2eaca2d..46bf8ccc8b 100644 --- a/poi/src/main/java/org/apache/poi/util/TempFile.java +++ b/poi/src/main/java/org/apache/poi/util/TempFile.java @@ -49,9 +49,10 @@ public final class TempFile { } /** - * Creates a new and empty temporary file. By default, files are collected into one directory and are - * deleted on exit from the VM, although they can be kept by defining the system property - * <code>poi.keep.tmp.files</code> (see {@link DefaultTempFileCreationStrategy}). + * Creates a new and empty temporary file. By default, files are collected into one directory and are not + * deleted on exit from the VM, although they can be deleted by defining the system property + * <code>poi.delete.tmp.files.on.exit</code> (see {@link DefaultTempFileCreationStrategy}), which may + * cause {@link OutOfMemoryError} problem due to the frequent usage of {@link File#deleteOnExit()} * <p> * Don't forget to close all files or it might not be possible to delete them. * |