Migrate UnixGlob to Path#statIfFound() instead of #statNullable(). The latter swallows all filesystem failures, and does not disambiguate missing files from filesystem problems.

The syscall cache now tracks IOExceptions if they are present, just as it does with readdir().

RELNOTES: None
PiperOrigin-RevId: 153185433

4 files changed, 72 insertions, 25 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/PathPackageLocator.java b/src/main/java/com/google/devtools/build/lib/pkgcache/PathPackageLocator.java
index 30609168f3..f828538419 100644
--- a/src/main/java/com/google/devtools/build/lib/pkgcache/PathPackageLocator.java
+++ b/src/main/java/com/google/devtools/build/lib/pkgcache/PathPackageLocator.java
@@ -28,6 +28,7 @@ import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.Symlinks;
 import com.google.devtools.build.lib.vfs.UnixGlob;
+import java.io.IOException;
 import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -111,10 +112,14 @@ public class PathPackageLocator implements Serializable {
       // invocation in Parser#include().
       Path buildFile = outputBase.getRelative(
           packageIdentifier.getSourceRoot()).getRelative("BUILD");
-      FileStatus stat = cache.get().statNullable(buildFile, Symlinks.FOLLOW);
-      if (stat != null && stat.isFile()) {
-        return buildFile;
-      } else {
+      try {
+        FileStatus stat = cache.get().statIfFound(buildFile, Symlinks.FOLLOW);
+        if (stat != null && stat.isFile()) {
+          return buildFile;
+        } else {
+          return null;
+        }
+      } catch (IOException e) {
         return null;
       }
     }
@@ -225,9 +230,13 @@ public class PathPackageLocator implements Serializable {
       AtomicReference<? extends UnixGlob.FilesystemCalls> cache) {
     for (Path pathEntry : pathEntries) {
       Path buildFile = pathEntry.getRelative(suffix);
-      FileStatus stat = cache.get().statNullable(buildFile, Symlinks.FOLLOW);
-      if (stat != null && stat.isFile()) {
-        return buildFile;
+      try {
+        FileStatus stat = cache.get().statIfFound(buildFile, Symlinks.FOLLOW);
+        if (stat != null && stat.isFile()) {
+          return buildFile;
+        }
+      } catch (IOException ignored) {
+        // Treat IOException as a missing file.
       }
     }
     return null;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PerBuildSyscallCache.java b/src/main/java/com/google/devtools/build/lib/skyframe/PerBuildSyscallCache.java
index 646d6093dd..d9eb4506a6 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PerBuildSyscallCache.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PerBuildSyscallCache.java
@@ -30,13 +30,14 @@ import java.util.Collection;
  */
 public class PerBuildSyscallCache implements UnixGlob.FilesystemCalls {
 
-  private final LoadingCache<Pair<Path, Symlinks>, FileStatus> statCache;
+  private final LoadingCache<Pair<Path, Symlinks>, Pair<FileStatus, IOException>> statCache;
   private final LoadingCache<Pair<Path, Symlinks>, Pair<Collection<Dirent>, IOException>>
       readdirCache;
 
   private static final FileStatus NO_STATUS = new FakeFileStatus();
 
-  private PerBuildSyscallCache(LoadingCache<Pair<Path, Symlinks>, FileStatus> statCache,
+  private PerBuildSyscallCache(
+      LoadingCache<Pair<Path, Symlinks>, Pair<FileStatus, IOException>> statCache,
       LoadingCache<Pair<Path, Symlinks>, Pair<Collection<Dirent>, IOException>> readdirCache) {
     this.statCache = statCache;
     this.readdirCache = readdirCache;
@@ -104,9 +105,12 @@ public class PerBuildSyscallCache implements UnixGlob.FilesystemCalls {
   }
 
   @Override
-  public FileStatus statNullable(Path path, Symlinks symlinks) {
-    FileStatus status = statCache.getUnchecked(Pair.of(path, symlinks));
-    return (status == NO_STATUS) ? null : status;
+  public FileStatus statIfFound(Path path, Symlinks symlinks) throws IOException {
+    Pair<FileStatus, IOException> status = statCache.getUnchecked(Pair.of(path, symlinks));
+    if (status.getFirst() != null) {
+      return (status.getFirst() == NO_STATUS) ? null : status.getFirst();
+    }
+    throw status.getSecond();
   }
 
   public void clear() {
@@ -162,12 +166,16 @@ public class PerBuildSyscallCache implements UnixGlob.FilesystemCalls {
    * Input: (path, following_symlinks)
    * Output: FileStatus
    */
-  private static CacheLoader<Pair<Path, Symlinks>, FileStatus> newStatLoader() {
-    return new CacheLoader<Pair<Path, Symlinks>, FileStatus>() {
+  private static CacheLoader<Pair<Path, Symlinks>, Pair<FileStatus, IOException>> newStatLoader() {
+    return new CacheLoader<Pair<Path, Symlinks>, Pair<FileStatus, IOException>>() {
         @Override
-        public FileStatus load(Pair<Path, Symlinks> p) {
-          FileStatus f = p.first.statNullable(p.second);
-          return (f == null) ? NO_STATUS : f;
+        public Pair<FileStatus, IOException> load(Pair<Path, Symlinks> p) {
+          try {
+            FileStatus f = p.first.statIfFound(p.second);
+            return Pair.of((f == null) ? NO_STATUS : f, null);
+          } catch (IOException e) {
+            return Pair.of(null, e);
+          }
         }
       };
   }
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/UnixGlob.java b/src/main/java/com/google/devtools/build/lib/vfs/UnixGlob.java
index d05677fe24..261072db75 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/UnixGlob.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/UnixGlob.java
@@ -271,7 +271,7 @@ public final class UnixGlob {
     /**
      * Return the stat() for the given path, or null.
      */
-    FileStatus statNullable(Path path, Symlinks symlinks);
+    FileStatus statIfFound(Path path, Symlinks symlinks) throws IOException;
   }
 
   public static FilesystemCalls DEFAULT_SYSCALLS = new FilesystemCalls() {
@@ -281,8 +281,8 @@ public final class UnixGlob {
     }
 
     @Override
-    public FileStatus statNullable(Path path, Symlinks symlinks) {
-      return path.statNullable(symlinks);
+    public FileStatus statIfFound(Path path, Symlinks symlinks) throws IOException {
+      return path.statIfFound(symlinks);
     }
   };
 
@@ -547,14 +547,19 @@ public final class UnixGlob {
      * Same as {@link #glob}, except does so asynchronously and returns a {@link Future} for the
      * result.
      */
-    public Future<List<Path>> globAsync(
+    Future<List<Path>> globAsync(
         Path base,
         Collection<String> patterns,
         boolean excludeDirectories,
         Predicate<Path> dirPred,
         FilesystemCalls syscalls) {
 
-      FileStatus baseStat = syscalls.statNullable(base, Symlinks.FOLLOW);
+      FileStatus baseStat;
+      try {
+        baseStat = syscalls.statIfFound(base, Symlinks.FOLLOW);
+      } catch (IOException e) {
+        return Futures.immediateFailedFuture(e);
+      }
       if (baseStat == null || patterns.isEmpty()) {
         return Futures.immediateFuture(Collections.<Path>emptyList());
       }
@@ -782,7 +787,7 @@ public final class UnixGlob {
       if (!pattern.contains("*") && !pattern.contains("?")) {
         // We do not need to do a readdir in this case, just a stat.
         Path child = base.getChild(pattern);
-        FileStatus status = context.syscalls.statNullable(child, Symlinks.FOLLOW);
+        FileStatus status = context.syscalls.statIfFound(child, Symlinks.FOLLOW);
         if (status == null || (!status.isDirectory() && !status.isFile())) {
           // The file is a dangling symlink, fifo, does not exist, etc.
           return;
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/GlobTest.java b/src/test/java/com/google/devtools/build/lib/vfs/GlobTest.java
index 430078ae86..c74e61d3b1 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/GlobTest.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/GlobTest.java
@@ -185,11 +185,36 @@ public class GlobTest {
   }
 
   @Test
+  public void testIOFailureOnStat() throws Exception {
+    UnixGlob.FilesystemCalls syscalls = new UnixGlob.FilesystemCalls() {
+      @Override
+      public FileStatus statIfFound(Path path, Symlinks symlinks) throws IOException {
+        throw new IOException("EIO");
+      }
+
+      @Override
+      public Collection<Dirent> readdir(Path path, Symlinks symlinks) {
+        throw new IllegalStateException();
+      }
+    };
+
+    try {
+      new UnixGlob.Builder(tmpPath)
+          .addPattern("foo/bar/wiz/file")
+          .setFilesystemCalls(new AtomicReference<>(syscalls))
+          .glob();
+      fail("Expected failure");
+    } catch (IOException e) {
+      assertThat(e).hasMessageThat().isEqualTo("EIO");
+    }
+  }
+
+  @Test
   public void testGlobWithoutWildcardsDoesNotCallReaddir() throws Exception {
     UnixGlob.FilesystemCalls syscalls = new UnixGlob.FilesystemCalls() {
       @Override
-      public FileStatus statNullable(Path path, Symlinks symlinks) {
-        return UnixGlob.DEFAULT_SYSCALLS.statNullable(path, symlinks);
+      public FileStatus statIfFound(Path path, Symlinks symlinks) throws IOException {
+        return UnixGlob.DEFAULT_SYSCALLS.statIfFound(path, symlinks);
       }
 
       @Override