gnodet commented on code in PR #364:
URL: https://github.com/apache/maven-resolver/pull/364#discussion_r1396043178
##########
maven-resolver-util/src/main/java/org/eclipse/aether/util/FileUtils.java:
##########
@@ -98,23 +108,59 @@ public static CollocatedTempFile newTempFile(Path file)
throws IOException {
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)) {
Review Comment:
This is not safe. The `CollocatedTempFile` should provide a way to create
an `OutputStream` so that it can be closed before the `CollocatedTempFile`.
This is needed so that buffered streams are fully written to disk before moving
the file.
See
https://github.com/apache/maven-resolver/blob/e721b01be9576396e36e4167da337180b44e38e9/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultFileProcessor.java#L90-L96
The code is using a `BufferedOutputStream` so that it may not be completely
written to the disk. I'm not sure if there's any guarantee on the ordering of
the `close()` methods in the `try/finally` block.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
