Fix IOUtils#fsync on Windows fsyncing directories (#43008)

Fsyncing directories on Windows is not possible. We always suppressed
this by allowing that an AccessDeniedException is thrown when attemping
to open the directory for reading. Yet, this suppression also allowed
other IOExceptions to be suppressed, and that was a bug (e.g., the
directory not existing, or a filesystem error and reasons that we might
get an access denied there, like genuine permissions issues). This
leniency was previously removed yet it exposed that we were suppressing
this case on Windows. Rather than relying on exceptions for flow control
and continuing to suppress there, we simply return early if attempting
to fsync a directory on Windows (we will not put this burden on the
caller).

Author: jasontedor

libs/core/src/main/java/org/elasticsearch/core/internal/io/IOUtils.java

@@ -24,6 +24,7 @@
 import java.nio.file.FileVisitResult;
 import java.nio.file.FileVisitor;
 import java.nio.file.Files;
+import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
 import java.nio.file.StandardOpenOption;
 import java.nio.file.attribute.BasicFileAttributes;
@@ -249,6 +250,7 @@ public FileVisitResult visitFileFailed(final Path file, final IOException exc) t
     }
 
     // TODO: replace with constants class if needed (cf. org.apache.lucene.util.Constants)
+    private static final boolean WINDOWS = System.getProperty("os.name").startsWith("Windows");
     private static final boolean LINUX = System.getProperty("os.name").startsWith("Linux");
     private static final boolean MAC_OS_X = System.getProperty("os.name").startsWith("Mac OS X");
 
@@ -263,6 +265,14 @@ public FileVisitResult visitFileFailed(final Path file, final IOException exc) t
      *                   systems and operating systems allow to fsync on a directory)
      */
     public static void fsync(final Path fileToSync, final boolean isDir) throws IOException {
+        if (isDir && WINDOWS) {
+            // opening a directory on Windows fails, directories can not be fsynced there
+            if (Files.exists(fileToSync) == false) {
+                // yet do not suppress trying to fsync directories that do not exist
+                throw new NoSuchFileException(fileToSync.toString());
+            }
+            return;
+        }
         try (FileChannel file = FileChannel.open(fileToSync, isDir ? StandardOpenOption.READ : StandardOpenOption.WRITE)) {
             try {
                 file.force(true);

libs/core/src/test/java/org/elasticsearch/core/internal/io/IOUtilsTests.java

@@ -19,6 +19,7 @@
 
 import org.apache.lucene.mockfile.FilterFileSystemProvider;
 import org.apache.lucene.mockfile.FilterPath;
+import org.apache.lucene.util.Constants;
 import org.elasticsearch.common.CheckedConsumer;
 import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.test.ESTestCase;
@@ -27,14 +28,20 @@
 import java.io.IOException;
 import java.io.OutputStream;
 import java.net.URI;
+import java.nio.channels.FileChannel;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.AccessDeniedException;
 import java.nio.file.FileSystem;
 import java.nio.file.Files;
+import java.nio.file.NoSuchFileException;
+import java.nio.file.OpenOption;
 import java.nio.file.Path;
+import java.nio.file.attribute.FileAttribute;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Objects;
+import java.util.Set;
 import java.util.function.Function;
 
 import static org.hamcrest.Matchers.arrayWithSize;
@@ -214,6 +221,43 @@ public void testFsyncDirectory() throws Exception {
         // no exception
     }
 
+    private static final class AccessDeniedWhileOpeningDirectoryFileSystem extends FilterFileSystemProvider {
+
+        AccessDeniedWhileOpeningDirectoryFileSystem(final FileSystem delegate) {
+            super("access_denied://", Objects.requireNonNull(delegate));
+        }
+
+        @Override
+        public FileChannel newFileChannel(
+                final Path path,
+                final Set<? extends OpenOption> options,
+                final FileAttribute<?>... attrs) throws IOException {
+            if (Files.isDirectory(path)) {
+                throw new AccessDeniedException(path.toString());
+            }
+            return delegate.newFileChannel(path, options, attrs);
+        }
+
+    }
+
+    public void testFsyncAccessDeniedOpeningDirectory() throws Exception {
+        final Path path = createTempDir().toRealPath();
+        final FileSystem fs = new AccessDeniedWhileOpeningDirectoryFileSystem(path.getFileSystem()).getFileSystem(URI.create("file:///"));
+        final Path wrapped = new FilterPath(path, fs);
+        if (Constants.WINDOWS) {
+            // no exception, we early return and do not even try to open the directory
+            IOUtils.fsync(wrapped, true);
+        } else {
+            expectThrows(AccessDeniedException.class, () -> IOUtils.fsync(wrapped, true));
+        }
+    }
+
+    public void testFsyncNonExistentDirectory() throws Exception {
+        final Path dir = FilterPath.unwrap(createTempDir()).toRealPath();
+        final Path nonExistentDir = dir.resolve("non-existent");
+        expectThrows(NoSuchFileException.class, () -> IOUtils.fsync(nonExistentDir, true));
+    }
+
     public void testFsyncFile() throws IOException {
         final Path path = createTempDir().toRealPath();
         final Path subPath = path.resolve(randomAlphaOfLength(8));