diff --git a/core/src/main/java/hudson/util/AtomicFileWriter.java b/core/src/main/java/hudson/util/AtomicFileWriter.java index fabe15c5a6b6..252605c06de7 100644 --- a/core/src/main/java/hudson/util/AtomicFileWriter.java +++ b/core/src/main/java/hudson/util/AtomicFileWriter.java @@ -67,6 +67,11 @@ public class AtomicFileWriter extends Writer { private static /* final */ boolean REQUIRES_DIR_FSYNC = SystemProperties.getBoolean( AtomicFileWriter.class.getName() + ".REQUIRES_DIR_FSYNC", !Functions.isWindows()); + /** + * Whether the platform supports atomic move. + */ + private static boolean atomicMoveSupported = true; + static { if (DISABLE_FORCED_FLUSH) { LOGGER.log(Level.WARNING, "DISABLE_FORCED_FLUSH flag used, this could result in dataloss if failures happen in your storage subsystem."); @@ -149,7 +154,7 @@ public AtomicFileWriter(@NonNull Path destinationPath, @NonNull Charset charset, try { // JENKINS-48407: NIO's createTempFile creates file with 0600 permissions, so we use pre-NIO for this... - tmpPath = File.createTempFile("atomic", "tmp", dir.toFile()).toPath(); + tmpPath = File.createTempFile(destPath.getFileName() + "-atomic", "tmp", dir.toFile()).toPath(); } catch (IOException e) { throw new IOException("Failed to create a temporary file in " + dir, e); } @@ -207,36 +212,14 @@ public void abort() throws IOException { public void commit() throws IOException { close(); try { - // Try to make an atomic move. - Files.move(tmpPath, destPath, StandardCopyOption.ATOMIC_MOVE); - } catch (IOException moveFailed) { - // If it falls here that can mean many things. Either that the atomic move is not supported, - // or something wrong happened. Anyway, let's try to be over-diagnosing - if (moveFailed instanceof AtomicMoveNotSupportedException) { - LOGGER.log(Level.WARNING, "Atomic move not supported. falling back to non-atomic move.", moveFailed); - } else { - LOGGER.log(Level.WARNING, "Unable to move atomically, falling back to non-atomic move.", moveFailed); - } - - if (destPath.toFile().exists()) { - LOGGER.log(Level.INFO, "The target file {0} was already existing", destPath); - } - + move(tmpPath, destPath); + } finally { try { - Files.move(tmpPath, destPath, StandardCopyOption.REPLACE_EXISTING); - } catch (IOException replaceFailed) { - replaceFailed.addSuppressed(moveFailed); - LOGGER.log(Level.WARNING, "Unable to move {0} to {1}. Attempting to delete {0} and abandoning.", - new Path[]{tmpPath, destPath}); - try { - Files.deleteIfExists(tmpPath); - } catch (IOException deleteFailed) { - replaceFailed.addSuppressed(deleteFailed); - LOGGER.log(Level.WARNING, "Unable to delete {0}, good bye then!", tmpPath); - throw replaceFailed; - } - - throw replaceFailed; + // In case of prior failure, the temporary file should be deleted. + // If the operation succeeded, the tmpPath is already deleted. + Files.deleteIfExists(tmpPath); + } catch (IOException e) { + LOGGER.log(Level.WARNING, e, () -> "Failed to delete temporary file " + tmpPath + " for destination file " + destPath); } } @@ -253,6 +236,23 @@ public void commit() throws IOException { } } + private static void move(Path source, Path destination) throws IOException { + if (atomicMoveSupported) { + try { + // Try to make an atomic move. + Files.move(source, destination, StandardCopyOption.ATOMIC_MOVE); + return; + } catch (AtomicMoveNotSupportedException e) { + // Both files are on the same filesystem, so this should not happen. + LOGGER.log(Level.WARNING, e, () -> "Atomic move " + source + " → " + destination + " not supported. Falling back to non-atomic move."); + atomicMoveSupported = false; + } + } + if (!atomicMoveSupported) { + Files.move(source, destination, StandardCopyOption.REPLACE_EXISTING); + } + } + private static final class CleanupChecker implements Runnable { private final FileChannelWriter core; private final Path tmpPath;