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)

Answer 1

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); }
});