[lucene-solr] branch branch_8x updated (00b146b -> 1ed2534)

[jp...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

jpountz pushed a change to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git.


    from 00b146b  Merge remote-tracking branch 'origin/branch_8x' into branch_8x
     new d7897e4  LUCENE-8843: Only ignore IOException on dirs when invoking force (#706)
     new 1ed2534  LUCENE-8843: Add CHANGES entry.

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 lucene/CHANGES.txt                                 |  3 ++
 .../src/java/org/apache/lucene/util/IOUtils.java   | 32 ++++++++++-----
 .../test/org/apache/lucene/util/TestIOUtils.java   | 45 ++++++++++++++++++++++
 3 files changed, 70 insertions(+), 10 deletions(-)

[lucene-solr] 02/02: LUCENE-8843: Add CHANGES entry.

[jp...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

jpountz pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit 1ed253460fe9d9722d31602c2a4f827862baff89
Author: Adrien Grand <jp...@gmail.com>
AuthorDate: Tue Jun 11 10:22:05 2019 +0200

    LUCENE-8843: Add CHANGES entry.
---
 lucene/CHANGES.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index d7c3e84..1942627 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -43,6 +43,9 @@ Bug Fixes
 * LUCENE-8828: Removes the buggy 'disallow overlaps' boolean from Intervals.unordered(),
   and replaces it with a new Intervals.unorderedNoOverlaps() method (Alan Woodward)
 
+* LUCENE-8843: Don't ignore exceptions that are thrown when trying to open a
+  file in IOUtils#fsync. (Jason Tedor via Adrien Grand)
+
 Improvements
 
 * LUCENE-7840: Non-scoring BooleanQuery now removes SHOULD clauses before building the scorer supplier

[lucene-solr] 01/02: LUCENE-8843: Only ignore IOException on dirs when invoking force (#706)

[jp...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

jpountz pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit d7897e4b1de74bc242a2d67fd8030007368b1594
Author: Jason Tedor <ja...@tedor.me>
AuthorDate: Tue Jun 11 04:19:14 2019 -0400

    LUCENE-8843: Only ignore IOException on dirs when invoking force (#706)
    
    Today in the method IOUtils#fsync we ignore IOExceptions when fsyncing a
    directory. However, the catch block here is too broad, for example it
    would be ignoring IOExceptions when we try to open a non-existent
    file. This commit addresses that by scoping the ignored exceptions only
    to the invocation of FileChannel#force. This prevents us from
    suppressing an exception in case we run into an unexpected issue when
    opening the file.
    
    However, 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, per the above, this
    suppression also allowed other IOExceptions to be suppressed, and that
    should be considered a bug (e.g., not only the directory not existing,
    but any filesystem error and other reasons that we might get an access
    denied there, like genuine permissions issues). 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 should
    not put this burden on the caller).
---
 .../src/java/org/apache/lucene/util/IOUtils.java   | 32 ++++++++++-----
 .../test/org/apache/lucene/util/TestIOUtils.java   | 45 ++++++++++++++++++++++
 2 files changed, 67 insertions(+), 10 deletions(-)

diff --git a/lucene/core/src/java/org/apache/lucene/util/IOUtils.java b/lucene/core/src/java/org/apache/lucene/util/IOUtils.java
index a556789..c4dfe10 100644
--- a/lucene/core/src/java/org/apache/lucene/util/IOUtils.java
+++ b/lucene/core/src/java/org/apache/lucene/util/IOUtils.java
@@ -18,6 +18,7 @@ package org.apache.lucene.util;
 
 import java.io.BufferedReader;
 import java.io.Closeable;
+import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
@@ -32,6 +33,7 @@ import java.nio.file.FileStore;
 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;
@@ -457,18 +459,28 @@ public final class IOUtils {
   public static void fsync(Path fileToSync, boolean isDir) throws IOException {
     // If the file is a directory we have to open read-only, for regular files we must open r/w for the fsync to have an effect.
     // See http://blog.httrack.com/blog/2013/11/15/everything-you-always-wanted-to-know-about-fsync/
+    if (isDir && Constants.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 (final FileChannel file = FileChannel.open(fileToSync, isDir ? StandardOpenOption.READ : StandardOpenOption.WRITE)) {
-      file.force(true);
-    } catch (IOException ioe) {
-      if (isDir) {
-        assert (Constants.LINUX || Constants.MAC_OS_X) == false :
-            "On Linux and MacOSX fsyncing a directory should not throw IOException, "+
-                "we just don't want to rely on that in production (undocumented). Got: " + ioe;
-        // Ignore exception if it is a directory
-        return;
+      try {
+        file.force(true);
+      } catch (final IOException e) {
+        if (isDir) {
+          assert (Constants.LINUX || Constants.MAC_OS_X) == false :
+              "On Linux and MacOSX fsyncing a directory should not throw IOException, " +
+                  "we just don't want to rely on that in production (undocumented). Got: " + e;
+          // Ignore exception if it is a directory
+          return;
+        }
+        // Throw original exception
+        throw e;
       }
-      // Throw original exception
-      throw ioe;
     }
   }
 
diff --git a/lucene/core/src/test/org/apache/lucene/util/TestIOUtils.java b/lucene/core/src/test/org/apache/lucene/util/TestIOUtils.java
index ec242a9..e01827f 100644
--- a/lucene/core/src/test/org/apache/lucene/util/TestIOUtils.java
+++ b/lucene/core/src/test/org/apache/lucene/util/TestIOUtils.java
@@ -19,12 +19,18 @@ package org.apache.lucene.util;
 
 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.FileStore;
 import java.nio.file.FileSystem;
 import java.nio.file.Files;
 import java.nio.file.LinkOption;
+import java.nio.file.NoSuchFileException;
+import java.nio.file.OpenOption;
 import java.nio.file.Path;
+import java.nio.file.attribute.FileAttribute;
 import java.nio.file.attribute.FileAttributeView;
 import java.nio.file.attribute.FileStoreAttributeView;
 import java.util.ArrayList;
@@ -34,7 +40,9 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Random;
+import java.util.Set;
 import java.util.UUID;
 
 import org.apache.lucene.mockfile.FilterFileSystem;
@@ -475,6 +483,43 @@ public class TestIOUtils extends LuceneTestCase {
     // 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 Exception {
     Path dir = createTempDir();
     dir = FilterPath.unwrap(dir).toRealPath();