Allow interrupts while accessing the cache

Certain operations in the repository cache take some time, e.g.,
verifying the checksum of a file. Allow interrupts while doing so.

While touching SkylarkRepositoryContext.java anyway, remove dead code there.

Fixes bazelbuild#8346

Change-Id: I26bbdb314554271949a34555600df1c35cc1544d
PiperOrigin-RevId: 249253196

src/main/java/com/google/devtools/build/lib/bazel/repository/MavenDownloader.java

@@ -64,7 +64,7 @@ public JarPaths download(
       Path outputDirectory,
       MavenServerValue serverValue,
       ExtendedEventHandler eventHandler)
-      throws IOException, EvalException {
+      throws IOException, EvalException, InterruptedException {
 
     String url = serverValue.getUrl();
     Server server = serverValue.getServer();

src/main/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunction.java

@@ -146,7 +146,7 @@ private RepositoryDirectoryValue.Builder createOutputTree(
       Path outputDirectory,
       MavenServerValue serverValue,
       ExtendedEventHandler eventHandler)
-      throws RepositoryFunctionException {
+      throws RepositoryFunctionException, InterruptedException {
     MavenDownloader mavenDownloader = downloader;
 
     createDirectory(outputDirectory);

src/main/java/com/google/devtools/build/lib/bazel/repository/cache/RepositoryCache.java

@@ -128,7 +128,7 @@ boolean hasCanonicalId(String cacheKey, KeyType keyType, String canonicalId) {
   }
 
   public synchronized Path get(String cacheKey, Path targetPath, KeyType keyType)
-      throws IOException {
+      throws IOException, InterruptedException {
     return get(cacheKey, targetPath, keyType, null);
   }
 
@@ -150,7 +150,11 @@ public synchronized Path get(String cacheKey, Path targetPath, KeyType keyType)
    */
   @Nullable
   public synchronized Path get(
-      String cacheKey, Path targetPath, KeyType keyType, String canonicalId) throws IOException {
+      String cacheKey, Path targetPath, KeyType keyType, String canonicalId)
+      throws IOException, InterruptedException {
+    if (Thread.interrupted()) {
+      throw new InterruptedException();
+    }
     Preconditions.checkState(isEnabled());
 
     assertKeyIsValid(cacheKey, keyType);
@@ -192,7 +196,7 @@ public synchronized Path get(
   }
 
   public synchronized void put(String cacheKey, Path sourcePath, KeyType keyType)
-      throws IOException {
+      throws IOException, InterruptedException {
     put(cacheKey, sourcePath, keyType, null);
   }
 
@@ -207,7 +211,12 @@ public synchronized void put(String cacheKey, Path sourcePath, KeyType keyType)
    * @throws IOException
    */
   public synchronized void put(
-      String cacheKey, Path sourcePath, KeyType keyType, String canonicalId) throws IOException {
+      String cacheKey, Path sourcePath, KeyType keyType, String canonicalId)
+      throws IOException, InterruptedException {
+    // Check for interrupts while waiting for the monitor of this synchronized method
+    if (Thread.interrupted()) {
+      throw new InterruptedException();
+    }
     Preconditions.checkState(isEnabled());
 
     assertKeyIsValid(cacheKey, keyType);
@@ -229,7 +238,8 @@ public synchronized void put(
     }
   }
 
-  public synchronized String put(Path sourcePath, KeyType keyType) throws IOException {
+  public synchronized String put(Path sourcePath, KeyType keyType)
+      throws IOException, InterruptedException {
     return put(sourcePath, keyType, null);
   }
 
@@ -244,7 +254,7 @@ public synchronized String put(Path sourcePath, KeyType keyType) throws IOExcept
    * @return The key for the cached entry.
    */
   public synchronized String put(Path sourcePath, KeyType keyType, String canonicalId)
-      throws IOException {
+      throws IOException, InterruptedException {
     String cacheKey = getChecksum(keyType, sourcePath);
     put(cacheKey, sourcePath, keyType, canonicalId);
     return cacheKey;
@@ -263,11 +273,11 @@ private void ensureCacheDirectoryExists(KeyType keyType) throws IOException {
    * @param expectedChecksum The expected checksum of the file.
    * @param filePath The path to the file.
    * @param keyType The type of hash function. e.g. SHA-1, SHA-256
-   * @throws IOException If the checksum does not match or the file cannot be hashed, an
-   *   exception is thrown.
+   * @throws IOException If the checksum does not match or the file cannot be hashed, an exception
+   *     is thrown.
    */
   public static void assertFileChecksum(String expectedChecksum, Path filePath, KeyType keyType)
-      throws IOException {
+      throws IOException, InterruptedException {
     Preconditions.checkArgument(!expectedChecksum.isEmpty());
 
     String actualChecksum;
@@ -292,7 +302,8 @@ public static void assertFileChecksum(String expectedChecksum, Path filePath, Ke
    * @param path The path to the file.
    * @throws IOException
    */
-  public static String getChecksum(KeyType keyType, Path path) throws IOException {
+  public static String getChecksum(KeyType keyType, Path path)
+      throws IOException, InterruptedException {
     Hasher hasher = keyType.newHasher();
     byte[] byteBuffer = new byte[BUFFER_SIZE];
     try (InputStream stream = path.getInputStream()) {
@@ -302,6 +313,9 @@ public static String getChecksum(KeyType keyType, Path path) throws IOException
           // If more than 0 bytes were read, add them to the hash.
           hasher.putBytes(byteBuffer, 0, numBytesRead);
         }
+        if (Thread.interrupted()) {
+          throw new InterruptedException();
+        }
         numBytesRead = stream.read(byteBuffer);
       }
     }

src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java

@@ -645,7 +645,8 @@ public StructImpl downloadAndExtract(
     return StructProvider.STRUCT.createStruct(dict, null);
   }
 
-  private String calculateSha256(String originalSha, Path path) throws IOException {
+  private String calculateSha256(String originalSha, Path path)
+      throws IOException, InterruptedException {
     if (!Strings.isNullOrEmpty(originalSha)) {
       // The sha is checked on download, so if we got here, the user provided sha is good
       return originalSha;
@@ -687,11 +688,6 @@ private static ImmutableList<String> checkAllUrls(Iterable<?> urlList) throws Ev
     return result.build();
   }
 
-  private static List<URL> getUrls(Object urlOrList)
-      throws RepositoryFunctionException, EvalException {
-    return getUrls(urlOrList, /* ensureNonEmpty= */ true);
-  }
-
   private static List<URL> getUrls(Object urlOrList, boolean ensureNonEmpty)
       throws RepositoryFunctionException, EvalException {
     List<String> urlStrings;

src/test/java/com/google/devtools/build/lib/bazel/repository/cache/RepositoryCacheTest.java

@@ -69,11 +69,9 @@ public void testNonExistentCacheValue() {
     assertThat(repositoryCache.exists(fakeSha256, KeyType.SHA256)).isFalse();
   }
 
-  /**
-   * Test that the put method correctly stores the downloaded file into the cache.
-   */
+  /** Test that the put method correctly stores the downloaded file into the cache. */
   @Test
-  public void testPutCacheValue() throws IOException {
+  public void testPutCacheValue() throws Exception {
     repositoryCache.put(downloadedFileSha256, downloadedFile, KeyType.SHA256);
 
     Path cacheEntry = KeyType.SHA256.getCachePath(contentAddressableCachePath).getChild(downloadedFileSha256);
@@ -87,7 +85,7 @@ public void testPutCacheValue() throws IOException {
    * Test that the put mehtod without cache key correctly stores the downloaded file into the cache.
    */
   @Test
-  public void testPutCacheValueWithoutHash() throws IOException {
+  public void testPutCacheValueWithoutHash() throws Exception {
     String cacheKey = repositoryCache.put(downloadedFile, KeyType.SHA256);
     assertThat(cacheKey).isEqualTo(downloadedFileSha256);
 
@@ -100,11 +98,11 @@ public void testPutCacheValueWithoutHash() throws IOException {
   }
 
   /**
-   * Test that the put method is idempotent, i.e. two successive put calls
-   * should not affect the final state in the cache.
+   * Test that the put method is idempotent, i.e. two successive put calls should not affect the
+   * final state in the cache.
    */
   @Test
-  public void testPutCacheValueIdempotent() throws IOException {
+  public void testPutCacheValueIdempotent() throws Exception {
     repositoryCache.put(downloadedFileSha256, downloadedFile, KeyType.SHA256);
     repositoryCache.put(downloadedFileSha256, downloadedFile, KeyType.SHA256);
 
@@ -115,11 +113,9 @@ public void testPutCacheValueIdempotent() throws IOException {
         .isEqualTo(FileSystemUtils.readContent(cacheValue, Charset.defaultCharset()));
   }
 
-  /**
-   * Test that the get method correctly retrieves the cached file from the cache.
-   */
+  /** Test that the get method correctly retrieves the cached file from the cache. */
   @Test
-  public void testGetCacheValue() throws IOException {
+  public void testGetCacheValue() throws Exception {
     // Inject file into cache
     repositoryCache.put(downloadedFileSha256, downloadedFile, KeyType.SHA256);
 
@@ -135,11 +131,9 @@ public void testGetCacheValue() throws IOException {
     assertThat((Object) actualTargetPath).isEqualTo(targetPath);
   }
 
-  /**
-   * Test that the get method retrieves a null if the value is not cached.
-   */
+  /** Test that the get method retrieves a null if the value is not cached. */
   @Test
-  public void testGetNullCacheValue() throws IOException {
+  public void testGetNullCacheValue() throws Exception {
     Path targetDirectory = scratch.dir("/external");
     Path targetPath = targetDirectory.getChild(downloadedFile.getBaseName());
     Path actualTargetPath = repositoryCache.get(downloadedFileSha256, targetPath, KeyType.SHA256);
@@ -148,15 +142,15 @@ public void testGetNullCacheValue() throws IOException {
   }
 
   @Test
-  public void testInvalidSha256Throws() throws IOException {
+  public void testInvalidSha256Throws() throws Exception {
     String invalidSha = "foo";
     thrown.expect(IOException.class);
     thrown.expectMessage("Invalid key \"foo\" of type SHA-256");
     repositoryCache.put(invalidSha, downloadedFile, KeyType.SHA256);
   }
 
   @Test
-  public void testPoisonedCache() throws IOException {
+  public void testPoisonedCache() throws Exception {
     Path poisonedEntry = KeyType.SHA256
         .getCachePath(contentAddressableCachePath).getChild(downloadedFileSha256);
     Path poisonedValue = poisonedEntry.getChild(RepositoryCache.DEFAULT_CACHE_FILENAME);
@@ -173,18 +167,18 @@ public void testPoisonedCache() throws IOException {
   }
 
   @Test
-  public void testGetChecksum() throws IOException {
+  public void testGetChecksum() throws Exception {
     String actualChecksum = RepositoryCache.getChecksum(KeyType.SHA256, downloadedFile);
     assertThat(actualChecksum).isEqualTo(downloadedFileSha256);
   }
 
   @Test
-  public void testAssertFileChecksumPass() throws IOException {
+  public void testAssertFileChecksumPass() throws Exception {
     RepositoryCache.assertFileChecksum(downloadedFileSha256, downloadedFile, KeyType.SHA256);
   }
 
   @Test
-  public void testAssertFileChecksumFail() throws IOException {
+  public void testAssertFileChecksumFail() throws Exception {
     thrown.expect(IOException.class);
     thrown.expectMessage("does not match expected");
     RepositoryCache.assertFileChecksum(