From d2a7522bc4e9d08f658977fa7613bfdb8772b1b5 Mon Sep 17 00:00:00 2001 From: Guillaume Raffin Date: Tue, 20 Aug 2024 15:23:47 +0200 Subject: [PATCH] core: Retry Files.move on AccessDeniedException, resolve #183 --- .../core/file/AsyncFileConfig.java | 25 ++-- .../nightconfig/core/io/ConfigWriter.java | 27 +++- .../nightconfig/core/io/IoUtils.java | 129 ++++++++++++++++++ 3 files changed, 163 insertions(+), 18 deletions(-) create mode 100644 core/src/main/java/com/electronwill/nightconfig/core/io/IoUtils.java diff --git a/core/src/main/java/com/electronwill/nightconfig/core/file/AsyncFileConfig.java b/core/src/main/java/com/electronwill/nightconfig/core/file/AsyncFileConfig.java index 0351ebb3..272e07c4 100644 --- a/core/src/main/java/com/electronwill/nightconfig/core/file/AsyncFileConfig.java +++ b/core/src/main/java/com/electronwill/nightconfig/core/file/AsyncFileConfig.java @@ -14,18 +14,12 @@ import java.time.Duration; import java.util.Collections; import java.util.List; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ThreadFactory; +import java.util.concurrent.*; import com.electronwill.nightconfig.core.*; import com.electronwill.nightconfig.core.concurrent.ConcurrentCommentedConfig; import com.electronwill.nightconfig.core.concurrent.StampedConfig; -import com.electronwill.nightconfig.core.io.ConfigParser; -import com.electronwill.nightconfig.core.io.ConfigWriter; -import com.electronwill.nightconfig.core.io.ParsingMode; -import com.electronwill.nightconfig.core.io.WritingException; -import com.electronwill.nightconfig.core.io.WritingMode; +import com.electronwill.nightconfig.core.io.*; import com.electronwill.nightconfig.core.utils.ConcurrentCommentedConfigWrapper; /** @@ -126,10 +120,19 @@ private void saveNow() { // The FileWriter is not kept open in that case, because the temporary file will no longer exist after the // move. if (writingMode == WritingMode.REPLACE_ATOMIC) { - Path tmp = nioPath.resolveSibling(nioPath.getFileName() + ".new.tmp"); + Path tmp = nioPath.resolveSibling(IoUtils.tempConfigFileName(nioPath)); try (BufferedWriter writer = Files.newBufferedWriter(tmp, charset, WRITE, CREATE, TRUNCATE_EXISTING)) { configWriter.write(copy, writer); - Files.move(tmp, nioPath, StandardCopyOption.ATOMIC_MOVE); + } catch (IOException e) { + String msg = String.format("Failed to write (%s) the config to: %s", + writingMode.toString(), tmp.toString()); + throw new WritingException(msg, e); + } + // Flush and close the output before atomically moving the file. + try { + IoUtils.retryIfAccessDenied("move", () -> { + Files.move(tmp, nioPath, StandardCopyOption.ATOMIC_MOVE); + }); } catch (AtomicMoveNotSupportedException e) { // can fail in some conditions (OS and filesystem-dependent) String msg = String.format( @@ -140,7 +143,7 @@ private void saveNow() { throw new WritingException(msg, e); } catch (IOException e) { // regular IO exception - String msg = String.format("Failed to write (%s) the config to: %s", + String msg = String.format("Failed to atomically write (%s) the config to: %s", writingMode.toString(), tmp.toString()); throw new WritingException(msg, e); } diff --git a/core/src/main/java/com/electronwill/nightconfig/core/io/ConfigWriter.java b/core/src/main/java/com/electronwill/nightconfig/core/io/ConfigWriter.java index e0df121f..d9e0420c 100644 --- a/core/src/main/java/com/electronwill/nightconfig/core/io/ConfigWriter.java +++ b/core/src/main/java/com/electronwill/nightconfig/core/io/ConfigWriter.java @@ -62,8 +62,9 @@ default void write(UnmodifiableConfig config, OutputStream output) { } /** - * Writes a configuration. The content of the file is overwritten. This method is equivalent to - * + * Writes a configuration. The content of the file is overwritten. This method + * is equivalent to + * * 
 	 * write(config, file, false)
 	 *
@@ -86,10 +87,21 @@ default void write(UnmodifiableConfig config, Path file, WritingMode writingMode default void write(UnmodifiableConfig config, Path file, WritingMode writingMode, Charset charset) { if (writingMode == WritingMode.REPLACE_ATOMIC) { // write to another file, then atomically move it - Path tmp = file.resolveSibling(file.getFileName().toString() + ".new.tmp"); + String tmpFileName = IoUtils.tempConfigFileName(file); + Path tmp = file.resolveSibling(tmpFileName); try (OutputStream output = Files.newOutputStream(tmp, WRITE, CREATE, TRUNCATE_EXISTING)) { write(config, output, charset); - Files.move(tmp, file, StandardCopyOption.ATOMIC_MOVE); + } catch (IOException e) { + // regular IO exception + String msg = String.format("Failed to write (%s) the config to: %s", + writingMode.toString(), file.toString()); + throw new WritingException(msg, e); + } + // Flush and close the output before atomically moving the file. + try { + IoUtils.retryIfAccessDenied("move", () -> { + Files.move(tmp, file, StandardCopyOption.ATOMIC_MOVE); + }); } catch (AtomicMoveNotSupportedException e) { // can fail in some conditions (OS and filesystem-dependent) String msg = String.format( @@ -100,7 +112,7 @@ default void write(UnmodifiableConfig config, Path file, WritingMode writingMode throw new WritingException(msg, e); } catch (IOException e) { // regular IO exception - String msg = String.format("Failed to write (%s) the config to: %s", + String msg = String.format("Failed to atomically write (%s) the config to: %s", writingMode.toString(), file.toString()); throw new WritingException(msg, e); } @@ -118,8 +130,9 @@ default void write(UnmodifiableConfig config, Path file, WritingMode writingMode } /** - * Writes a configuration. The content of the file is overwritten. This method is equivalent to - * + * Writes a configuration. The content of the file is overwritten. This method + * is equivalent to + * * 
 	 * write(config, file, false)
 	 *
diff --git a/core/src/main/java/com/electronwill/nightconfig/core/io/IoUtils.java b/core/src/main/java/com/electronwill/nightconfig/core/io/IoUtils.java new file mode 100644 index 00000000..fbb4e4a1 --- /dev/null +++ b/core/src/main/java/com/electronwill/nightconfig/core/io/IoUtils.java @@ -0,0 +1,129 @@ +package com.electronwill.nightconfig.core.io; + +import java.io.IOException; +import java.nio.file.AccessDeniedException; +import java.nio.file.Path; +import java.util.concurrent.TimeUnit; + +/** + * IO utilities for INTERNAL use only (do not use oustide of night-config). + */ +public final class IoUtils { + /** + * Like {@code Runnable}, but with a throwable {@code IOException}. + */ + @FunctionalInterface + public static interface IoRunnable { + void run() throws IOException; + } + + public static final class RetryFailedException extends IOException { + RetryFailedException(String msg, IOException cause) { + super(msg, cause); + } + } + + static class OptionHolder { + static final long RETRY_DELAY_MILLIS; + static final int RETRY_MAX_TIMES; + + static { + boolean isWindows = System.getProperty("os.name", "?").trim().toLowerCase().startsWith("Windows"); + // Default max delay (delay*times) per OS (chosen arbitrarily, knowing that most + // issues happen on Windows): + // - Windows: 1.5s (3 retrys) + // - Others: 0.5s (1 try + 500ms delay + 1 retry) + + String delayProps = System.getProperty("nightconfig.accessDeniedRetryDelayMillis", "?"); + long delay; + try { + delay = Long.parseLong(delayProps); + } catch (NumberFormatException ex) { + delay = 500; + } + + String timesProps = System.getProperty("nightconfig.accessDeniedRetryMaxTimes", "?"); + int times; + try { + times = Integer.parseInt(timesProps); + } catch (NumberFormatException ex) { + times = isWindows ? 3 : 1; + } + + RETRY_DELAY_MILLIS = delay; + RETRY_MAX_TIMES = times; + } + } + + static String[] splitOnce(String s, char c) { + int i = s.lastIndexOf(c); + if (i < 0) { + return new String[] { s }; + } else { + return new String[] { s.substring(0, i), s.substring(i + 1, s.length()) }; + } + } + + /** + * Generates a filename (not path) for a temporary config file, for use with {@link WritingMode#REPLACE_ATOMIC}. + * Tries to keep the extension of the original file, to make it easier to find config files and to add them to + * file scanning whitelist (see the issue related to Windows Defender locking config files). + * + * @param originalFile the original config file + * @return a filename for the temporary file (the file is not created by this method) + */ + public static String tempConfigFileName(Path originalFile) { + String filename = originalFile.toString(); + String[] parts = splitOnce(filename, '.'); + if (parts.length == 1) { + return filename + ".new.tmp"; + } else { + return parts[0] + ".new.tmp." + parts[1]; + } + } + + /** + * Run an IO operation and retry it (at most {@code maxRetries} retries) if it + * fails with {@code AccessDeniedException}. + * See https://github.com/TheElectronWill/night-config/issues/183. + * + * @param name a name to print in error messages + * @param r the operation to run + * @throws IOException if it fails after retrying too many times, or for an error other than {@code AccessDeniedException} + */ + public static void retryIfAccessDenied(String name, IoRunnable r) throws IOException { + // load the default values from java properties, keep them in memory once loaded + retryIfAccessDenied( + name, + r, + OptionHolder.RETRY_MAX_TIMES, + OptionHolder.RETRY_DELAY_MILLIS, + TimeUnit.MILLISECONDS + ); + } + + public static void retryIfAccessDenied(String name, IoRunnable r, int maxRetries, long retryDelay, + TimeUnit delayUnit) + throws IOException { + AccessDeniedException lastException = null; + for (int i = 0; i <= maxRetries; i++) { + try { + r.run(); + return; + } catch (AccessDeniedException ex) { + // The file may be locked by another application (like an antivirus), + // try again after some time + lastException = ex; + try { + Thread.sleep(delayUnit.toMillis(retryDelay)); + } catch (InterruptedException e) { + // ignore + } + } catch (IOException ex) { + throw ex; + } + } + String msg = String.format("IO operation '%s' failed after %s attempts", name, maxRetries); + throw new RetryFailedException(msg, lastException); + } +}