From c2f302e7c3f9ad2a5958a62e3ce4e457271f292c Mon Sep 17 00:00:00 2001 From: Patrick Reinhart Date: Tue, 12 Sep 2023 08:05:35 +0200 Subject: [PATCH] [cleanup] Removed finalize() methods Removed all finalize() methods for the following classes: - exist-core/src/main/java/org/exist/util/GZIPInputSource.java - exist-core/src/main/java/org/exist/xmldb/AbstractRemoteResource.java - exist-core/src/main/java/org/exist/xmldb/RemoteResourceSet.java - exist-core/src/main/java/org/exist/xmlrpc/AbstractCachedResult.java Remplaced finalize() method with shutdown hook: - exist-core/src/main/java/org/exist/util/io/TemporaryFileManager.java Signed-off-by: Patrick Reinhart --- .../java/org/exist/util/GZIPInputSource.java | 9 --- .../exist/util/io/TemporaryFileManager.java | 60 ++++++++----------- .../exist/xmldb/AbstractRemoteResource.java | 9 --- .../org/exist/xmldb/RemoteResourceSet.java | 9 --- .../exist/xmlrpc/AbstractCachedResult.java | 10 ---- 5 files changed, 25 insertions(+), 72 deletions(-) diff --git a/exist-core/src/main/java/org/exist/util/GZIPInputSource.java b/exist-core/src/main/java/org/exist/util/GZIPInputSource.java index 3d3828e5943..409b7d9416a 100644 --- a/exist-core/src/main/java/org/exist/util/GZIPInputSource.java +++ b/exist-core/src/main/java/org/exist/util/GZIPInputSource.java @@ -144,15 +144,6 @@ public String getSymbolicPath() { return gzipFile.toAbsolutePath().toString(); } - @Override - protected void finalize() throws Throwable { - try { - close(); - } finally { - super.finalize(); - } - } - @Override public void close() { if(!isClosed()) { diff --git a/exist-core/src/main/java/org/exist/util/io/TemporaryFileManager.java b/exist-core/src/main/java/org/exist/util/io/TemporaryFileManager.java index 04818ab8200..b8043ae4d31 100644 --- a/exist-core/src/main/java/org/exist/util/io/TemporaryFileManager.java +++ b/exist-core/src/main/java/org/exist/util/io/TemporaryFileManager.java @@ -31,12 +31,15 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.nio.file.StandardOpenOption; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.exist.util.FileUtils; +import static java.nio.file.StandardOpenOption.CREATE_NEW; +import static java.nio.file.StandardOpenOption.DELETE_ON_CLOSE; +import static java.nio.file.StandardOpenOption.WRITE; + /** * Temporary File Manager. * @@ -64,7 +67,7 @@ * and the Operating System to manage it's temporary file space. * * Relevant articles on the above described problems are: - * 1.https://bugs.java.com/view_bug.do?bug_id=4715154 + * 1. https://bugs.java.com/view_bug.do?bug_id=4715154 * 2. https://bugs.openjdk.java.net/browse/JDK-8028683 * 3. https://bugs.java.com/view_bug.do?bug_id=4724038 * @@ -90,30 +93,33 @@ public static TemporaryFileManager getInstance() { private TemporaryFileManager() { cleanupOldTempFolders(); - try { this.tmpFolder = Files.createTempDirectory(FOLDER_PREFIX + '-'); - this.lockChannel = FileChannel.open(tmpFolder.resolve(LOCK_FILENAME), StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE, StandardOpenOption.DELETE_ON_CLOSE); + this.lockChannel = FileChannel.open(tmpFolder.resolve(LOCK_FILENAME), CREATE_NEW, WRITE, DELETE_ON_CLOSE); lockChannel.lock(); + Runtime.getRuntime().addShutdownHook(new Thread(this::cleanUpTempFolders)); } catch(final IOException ioe) { throw new RuntimeException("Unable to create temporary folder", ioe); } + LOG.info("Temporary folder is: {}", tmpFolder); + } - /* - Add hook to JVM to delete the file on exit - unfortunately this does not always work on all (e.g. Windows) platforms - will be recovered on restart by cleanupOldTempFolders - */ - tmpFolder.toFile().deleteOnExit(); - - LOG.info("Temporary folder is: {}", tmpFolder.toAbsolutePath().toString()); + private void cleanUpTempFolders() { + LOG.info("Removing temporary folder is: {}", tmpFolder); + try { + lockChannel.close(); // will release the lock on the lock file, and the lock file should be deleted + //try and remove our temporary folder + FileUtils.deleteQuietly(tmpFolder); + } catch(final IOException ioe) { + LOG.error("Feiled to cleanup {}", tmpFolder, ioe); + } } public final Path getTemporaryFile() throws IOException { // Be sure that the temp directory exists, create otherwise. #3826 if (!Files.exists(tmpFolder)) { - LOG.debug("Recreating {}", tmpFolder.toAbsolutePath()); + LOG.debug("Recreating {}", tmpFolder); Files.createDirectories(tmpFolder); } @@ -131,13 +137,11 @@ unfortunately this does not always work on all (e.g. Windows) platforms public void returnTemporaryFile(final Path tempFile) { try { if (Files.deleteIfExists(tempFile)) { - if (LOG.isDebugEnabled()) { - LOG.debug("Deleted temporary file: {}", tempFile.toAbsolutePath().toString()); - } + LOG.debug("Deleted temporary file: {}", tempFile); } } catch (final IOException e) { // this can often occur on Microsoft Windows (especially if the file was memory mapped!) :-/ - LOG.warn("Unable to delete temporary file: {} due to: {}", tempFile.toAbsolutePath().toString(), e.getMessage()); + LOG.warn("Unable to delete temporary file: {} due to: {}", tempFile, e.getMessage()); } } @@ -148,39 +152,25 @@ public void returnTemporaryFile(final Path tempFile) { */ private void cleanupOldTempFolders() { final Path tmpDir = Paths.get(System.getProperty("java.io.tmpdir")); - try { - for(final Path dir : FileUtils.list(tmpDir, path -> Files.isDirectory(path) && path.startsWith(FOLDER_PREFIX))) { + for (final Path dir : FileUtils.list(tmpDir, path -> Files.isDirectory(path) && path.startsWith(FOLDER_PREFIX))) { final Path lockPath = dir.resolve(LOCK_FILENAME); - if(!Files.exists(lockPath)) { + if (!Files.exists(lockPath)) { // no lock file present, so not in use FileUtils.deleteQuietly(dir); } else { // there is a lock file present, we must determine if it is locked (by another eXist-db instance) - try (final FileChannel otherLockChannel = FileChannel.open(lockPath, StandardOpenOption.WRITE)) { + try (final FileChannel otherLockChannel = FileChannel.open(lockPath, WRITE)) { if (otherLockChannel.tryLock() != null) { // not locked... so we now have the lock - FileUtils.deleteQuietly(dir); } } // will release the lock } } - } catch(final IOException ioe) { + } catch (final IOException ioe) { LOG.warn("Unable to delete old temporary folders", ioe); } } - - @Override - protected void finalize() throws Throwable { - try { - lockChannel.close(); // will release the lock on the lock file, and the lock file should be deleted - - //try and remove our temporary folder - FileUtils.deleteQuietly(tmpFolder); - } finally { - super.finalize(); - } - } } diff --git a/exist-core/src/main/java/org/exist/xmldb/AbstractRemoteResource.java b/exist-core/src/main/java/org/exist/xmldb/AbstractRemoteResource.java index 7f679a307ac..5d4237bb3fe 100644 --- a/exist-core/src/main/java/org/exist/xmldb/AbstractRemoteResource.java +++ b/exist-core/src/main/java/org/exist/xmldb/AbstractRemoteResource.java @@ -566,13 +566,4 @@ public final void close() { public final void freeResources() { close(); } - - @Override - protected void finalize() throws Throwable { - try { - close(); - } finally { - super.finalize(); - } - } } diff --git a/exist-core/src/main/java/org/exist/xmldb/RemoteResourceSet.java b/exist-core/src/main/java/org/exist/xmldb/RemoteResourceSet.java index 2dc6a2b24a8..cd610fd5aff 100644 --- a/exist-core/src/main/java/org/exist/xmldb/RemoteResourceSet.java +++ b/exist-core/src/main/java/org/exist/xmldb/RemoteResourceSet.java @@ -304,15 +304,6 @@ public final void close() throws XMLDBException { } } - @Override - protected void finalize() throws Throwable { - try { - close(); - } finally { - super.finalize(); - } - } - class NewResourceIterator implements ResourceIterator { long pos = 0; diff --git a/exist-core/src/main/java/org/exist/xmlrpc/AbstractCachedResult.java b/exist-core/src/main/java/org/exist/xmlrpc/AbstractCachedResult.java index 86ad37b6030..44287089f8f 100644 --- a/exist-core/src/main/java/org/exist/xmlrpc/AbstractCachedResult.java +++ b/exist-core/src/main/java/org/exist/xmlrpc/AbstractCachedResult.java @@ -128,14 +128,4 @@ public final void close() { public final void free() { close(); } - - @Override - protected void finalize() throws Throwable { - // Calling free to reclaim pinned resources - try { - close(); - } finally { - super.finalize(); - } - } }