This is an automated email from the ASF dual-hosted git repository.
cstamas pushed a commit to branch maven-resolver-1.9.x
in repository https://gitbox.apache.org/repos/asf/maven-resolver.git
The following commit(s) were added to refs/heads/maven-resolver-1.9.x by this
push:
new 1b8ce7f9 [MRESOLVER-372] Rework the FileUtils collocated temp file
(#365)
1b8ce7f9 is described below
commit 1b8ce7f95bd025024f5d0a4023b338147001040b
Author: Tamas Cservenak <ta...@cservenak.net>
AuthorDate: Fri Nov 17 13:27:18 2023 +0100
[MRESOLVER-372] Rework the FileUtils collocated temp file (#365)
Fixes:
* move() call should NOT perform the move, as writer stream to tmp file may
still be open
* move the file move logic to close, make it happen only when closing
collocated temp file
* perform fsync before atomic move to ensure there is no OS dirty buffers
related to newly written file
* on windows go with old code that for some reason works (avoid NIO2)
* on non-Win OS fsync the parent directory as well.
---
https://issues.apache.org/jira/browse/MRESOLVER-372
Backport to 1.9.x branch of the
https://github.com/apache/maven-resolver/pull/364
---
.../java/org/eclipse/aether/util/FileUtils.java | 75 +++++++++++++++++++++-
1 file changed, 72 insertions(+), 3 deletions(-)
diff --git
a/maven-resolver-util/src/main/java/org/eclipse/aether/util/FileUtils.java
b/maven-resolver-util/src/main/java/org/eclipse/aether/util/FileUtils.java
index dc260d76..0ec3880b 100644
--- a/maven-resolver-util/src/main/java/org/eclipse/aether/util/FileUtils.java
+++ b/maven-resolver-util/src/main/java/org/eclipse/aether/util/FileUtils.java
@@ -20,10 +20,16 @@ package org.eclipse.aether.util;
import java.io.Closeable;
import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.nio.ByteBuffer;
+import java.nio.channels.FileChannel;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
+import java.nio.file.StandardOpenOption;
import java.util.concurrent.ThreadLocalRandom;
+import java.util.concurrent.atomic.AtomicBoolean;
import static java.util.Objects.requireNonNull;
@@ -33,6 +39,10 @@ import static java.util.Objects.requireNonNull;
* @since 1.9.0
*/
public final class FileUtils {
+ // Logic borrowed from Commons-Lang3: we really need only this, to decide
do we fsync on directories or not
+ private static final boolean IS_WINDOWS =
+ System.getProperty("os.name", "unknown").startsWith("Windows");
+
private FileUtils() {
// hide constructor
}
@@ -52,7 +62,10 @@ public final class FileUtils {
*/
public interface CollocatedTempFile extends TempFile {
/**
- * Atomically moves temp file to target file it is collocated with.
+ * Upon close, atomically moves temp file to target file it is
collocated with overwriting target (if exists).
+ * Invocation of this method merely signals that caller ultimately
wants temp file to replace the target
+ * file, but when this method returns, the move operation did not yet
happen, it will happen when this
+ * instance is closed.
*/
void move() throws IOException;
}
@@ -98,23 +111,79 @@ public final class FileUtils {
Path tempFile = parent.resolve(file.getFileName() + "."
+
Long.toUnsignedString(ThreadLocalRandom.current().nextLong()) + ".tmp");
return new CollocatedTempFile() {
+ private final AtomicBoolean wantsMove = new AtomicBoolean(false);
+
@Override
public Path getPath() {
return tempFile;
}
@Override
- public void move() throws IOException {
- Files.move(tempFile, file, StandardCopyOption.ATOMIC_MOVE);
+ public void move() {
+ wantsMove.set(true);
}
@Override
public void close() throws IOException {
+ if (wantsMove.get() && Files.isReadable(tempFile)) {
+ if (IS_WINDOWS) {
+ copy(tempFile, file);
+ } else {
+ fsyncFile(tempFile);
+ Files.move(tempFile, file,
StandardCopyOption.ATOMIC_MOVE);
+ fsyncParent(tempFile);
+ }
+ }
Files.deleteIfExists(tempFile);
}
};
}
+ /**
+ * On Windows we use pre-NIO2 way to copy files, as for some reason it
works. Beat me why.
+ */
+ private static void copy(Path source, Path target) throws IOException {
+ ByteBuffer buffer = ByteBuffer.allocate(1024 * 32);
+ byte[] array = buffer.array();
+ try (InputStream is = Files.newInputStream(source);
+ OutputStream os = Files.newOutputStream(target)) {
+ while (true) {
+ int bytes = is.read(array);
+ if (bytes < 0) {
+ break;
+ }
+ os.write(array, 0, bytes);
+ }
+ }
+ }
+
+ /**
+ * Performs fsync: makes sure no OS "dirty buffers" exist for given file.
+ *
+ * @param target Path that must not be {@code null}, must exist as plain
file.
+ */
+ private static void fsyncFile(Path target) throws IOException {
+ try (FileChannel file = FileChannel.open(target,
StandardOpenOption.WRITE)) {
+ file.force(true);
+ }
+ }
+
+ /**
+ * Performs directory fsync: not usable on Windows, but some other OSes
may also throw, hence thrown IO exception
+ * is just ignored.
+ *
+ * @param target Path that must not be {@code null}, must exist as plain
file, and must have parent.
+ */
+ private static void fsyncParent(Path target) throws IOException {
+ try (FileChannel parent = FileChannel.open(target.getParent(),
StandardOpenOption.READ)) {
+ try {
+ parent.force(true);
+ } catch (IOException e) {
+ // ignore
+ }
+ }
+ }
+
/**
* A file writer, that accepts a {@link Path} to write some content to.
Note: the file denoted by path may exist,
* hence implementation have to ensure it is able to achieve its goal
("replace existing" option or equivalent
