commit 9cb225addbef0a695acc13dcf85a2b4cda600018
author George Gensure <ggensure@uber.com> Mon Sep 23 05:56:27 2019 -0700
committer Dmitry Lomov <dslomov@google.com> Mon Oct 14 14:50:28 2019 +0200
tree 74f5f55c5728f3d0580793e77246f7b2a28e1235
parent 9c257df5b9b1de5459fc066e77c435ae085957a7

Prevent action-cache duplicate suppression

Exceptions that are reused between download futures must not be added to
the suppression of themselves.

Fixes #9362

Closes #9376.

PiperOrigin-RevId: 270658205

src/main/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCache.java

diff --git a/src/main/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCache.java b/src/main/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCache.java
index 95f61b5..59d6f5a 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCache.java
@@ -354,13 +354,13 @@
       } catch (IOException e) {
         if (downloadException == null) {
           downloadException = e;
-        } else {
+        } else if (e != downloadException) {
           downloadException.addSuppressed(e);
         }
       } catch (InterruptedException e) {
         if (interruptedException == null) {
           interruptedException = e;
-        } else {
+        } else if (e != interruptedException) {
           interruptedException.addSuppressed(e);
         }
       }

src/test/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCacheTests.java

diff --git a/src/test/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCacheTests.java b/src/test/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCacheTests.java
index 141a4dd..d8bc54a 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCacheTests.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCacheTests.java
@@ -717,6 +717,64 @@
   }
 
   @Test
+  public void downloadWithDuplicateIOErrorsDoesNotSuppress() throws Exception {
+    Path stdout = fs.getPath("/execroot/stdout");
+    Path stderr = fs.getPath("/execroot/stderr");
+
+    DefaultRemoteActionCache cache = newTestCache();
+    Digest digest1 = cache.addContents("file1");
+    IOException reusedException = new IOException("reused io exception");
+    Digest digest2 = cache.addException("file2", reusedException);
+    Digest digest3 = cache.addException("file3", reusedException);
+
+    ActionResult result =
+        ActionResult.newBuilder()
+            .setExitCode(0)
+            .addOutputFiles(OutputFile.newBuilder().setPath("file1").setDigest(digest1))
+            .addOutputFiles(OutputFile.newBuilder().setPath("file2").setDigest(digest2))
+            .addOutputFiles(OutputFile.newBuilder().setPath("file3").setDigest(digest3))
+            .build();
+    IOException e =
+        assertThrows(
+            IOException.class,
+            () ->
+                cache.download(
+                    result, execRoot, new FileOutErr(stdout, stderr), outputFilesLocker));
+
+    assertThat(e.getSuppressed()).isEmpty();
+    assertThat(Throwables.getRootCause(e)).hasMessageThat().isEqualTo("reused io exception");
+  }
+
+  @Test
+  public void downloadWithDuplicateInterruptionsDoesNotSuppress() throws Exception {
+    Path stdout = fs.getPath("/execroot/stdout");
+    Path stderr = fs.getPath("/execroot/stderr");
+
+    DefaultRemoteActionCache cache = newTestCache();
+    Digest digest1 = cache.addContents("file1");
+    InterruptedException reusedInterruption = new InterruptedException("reused interruption");
+    Digest digest2 = cache.addException("file2", reusedInterruption);
+    Digest digest3 = cache.addException("file3", reusedInterruption);
+
+    ActionResult result =
+        ActionResult.newBuilder()
+            .setExitCode(0)
+            .addOutputFiles(OutputFile.newBuilder().setPath("file1").setDigest(digest1))
+            .addOutputFiles(OutputFile.newBuilder().setPath("file2").setDigest(digest2))
+            .addOutputFiles(OutputFile.newBuilder().setPath("file3").setDigest(digest3))
+            .build();
+    InterruptedException e =
+        assertThrows(
+            InterruptedException.class,
+            () ->
+                cache.download(
+                    result, execRoot, new FileOutErr(stdout, stderr), outputFilesLocker));
+
+    assertThat(e.getSuppressed()).isEmpty();
+    assertThat(Throwables.getRootCause(e)).hasMessageThat().isEqualTo("reused interruption");
+  }
+
+  @Test
   public void testDownloadWithStdoutStderrOnSuccess() throws Exception {
     // Tests that fetching stdout/stderr as a digest works and that OutErr is still
     // writable afterwards.