A bit strange behaviour of Files.delete and Files.deleteIfExists

Question

I got code like this:

paths.forEach(folderPath -> {
        Path to = folderPath.getRoot().resolve(folderPath.getParent().subpath(0, folderPath.getNameCount() - 1)); // До имени (исключительно)
        try {
            Files.list(folderPath).forEach(filePath -> {
                try { Files.move(filePath, to.resolve(filePath.getFileName()), StandardCopyOption.ATOMIC_MOVE); }
                catch (IOException e) { processException(e); }
            });
            if (Files.list(folderPath).count() == 0)
                Files.deleteIfExists(folderPath); // this call
        } catch (IOException e) { processException(e); }
    });

After I call delete methods, I get my empty directory locked (right after it was called, checked it), but not deleted until application is closed. I find it a bit strange, but want to know why is this happening.

(I use Windows 10)

*get my empty directory locked* you might want to explain what kind of lock do you mean here? Is this behavior specific to Java-9? — Naman, Jan 18 '18 at 02:43
@kleopatra I use it in JavaFX project, so mentioned it just in case. — Aqluse, Jan 18 '18 at 07:55
@nullpointer Didn't checked it. I run it on 9.0.1 The lock in my situation means that directory still visible but when I try to do something with it (for example see contents), It gives me a warning that I cannot get access to it. — Aqluse, Jan 18 '18 at 07:55
@Holger It is Path for moving file to parent directory of current directory. After all files are moved, program deletes directory — Aqluse, Jan 18 '18 at 09:39

Holger · Accepted Answer · 2018-01-18T13:41:23.370

From the documentation of Files.list(Path):

This method must be used within a try-with-resources statement or similar control structure to ensure that the stream's open directory is closed promptly after the stream's operations have completed.

You are not doing this, so the following part of Files.deleteIfExists(…) applies:

On some operating systems it may not be possible to remove a file when it is open and in use by this Java virtual machine or other programs.

You should use

paths.forEach(folderPath -> {
    Path to = folderPath.getParent();
    try {
        try(Stream<Path> files = Files.list(folderPath)) {
            files.forEach(filePath -> {
                try{Files.move(filePath, to.resolve(filePath.getFileName()), ATOMIC_MOVE);}
                catch (IOException e) { processException(e); }
            });
        }
        try {
            Files.deleteIfExists(folderPath);
        } catch(DirectoryNotEmptyException ex) {
            // may happen as you continue when Files.move fails,
            // but you already reported the original exception then
        }
    } catch (IOException e) { processException(e); }
});

This closes the stream of files before trying to delete the directory. Note that the second stream operation has been removed, this kind of pre-check is wasteful and should be unneeded when all move operations succeeded. But if some other application inserts a new file concurrently, there is no guaranty that it doesn’t happen between your Files.list(folderPath).count() == 0 check and the subsequent deleteIfExists call.

The cleaner solution would be to remember when a move failed. When no move failed, a still not empty directory should be considered an erroneous situation that should be reported like any other error, e.g.

paths.forEach(folderPath -> {
    Path to = folderPath.getParent();
    try {
        boolean allMovesSucceeded;
        try(Stream<Path> files = Files.list(folderPath)) {
          allMovesSucceeded = files
            .map(filePath -> {
                try {
                    Files.move(filePath, to.resolve(filePath.getFileName()), ATOMIC_MOVE);
                    return true;
                }
                catch(IOException e) { processException(e); return false; }
            }).reduce(Boolean.TRUE, Boolean::logicalAnd);

        }
        if(allMovesSucceeded) Files.deleteIfExists(folderPath);
    } catch (IOException e) { processException(e); }
});

I was surprised, but your fix and if pull out deletion code from try-with-resources work the same - with locking directory. — Aqluse, Jan 18 '18 at 13:28
Actually, my intention was to free the stream directly before the `deleteIfExists`, having delete after try-with-resources, but I mixed the scopes. But even with the wrong scopes, the directory was locked when calling `deleteIfExists`, but got deleted right when the stream was closed at the end of the try-with-resources block, which was still much earlier than in your original code where it got closed on JVM exit (or by the garbage collector at some unspecified time long after executing this code). — Holger, Jan 18 '18 at 13:46

A bit strange behaviour of Files.delete and Files.deleteIfExists

1 Answers1