Be resilient to the case where the worker gets closed due to memory p…

…ressure while the host Skyframe thread is inactive

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

@@ -75,7 +75,7 @@ class RepoFetchingSkyKeyComputeState implements SkyKeyComputeState {
   /** The executor service that manages the worker thread. */
   // We hold on to this alongside `workerFuture` because it offers a convenient mechanism to make
   // sure the worker thread has shut down (with its blocking `close()` method).
-  private ListeningExecutorService workerExecutorService;
+  ListeningExecutorService workerExecutorService;
 
   private final String repoName;
 
@@ -136,6 +136,7 @@ public void close() {
     if (myWorkerFuture != null) {
       myWorkerFuture.cancel(true);
     }
+    workerExecutorService.shutdownNow();
   }
 
   /**

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

@@ -140,6 +140,17 @@ public RepositoryDirectoryValue.Builder fetch(
       return fetchInternal(args);
     }
     var state = env.getState(() -> new RepoFetchingSkyKeyComputeState(rule.getName()));
+    if (state.workerExecutorService.isShutdown()) {
+      // If we get here and the worker executor is shut down, this can only mean that the worker
+      // future was cancelled while we (the host Skyframe thread) were inactive (as in, having
+      // returned `null` but not yet restarted). So we wait for the previous worker thread to finish
+      // first.
+      // TODO: instead of this complicated dance, consider making it legal for
+      //  `SkyKeyComputeState#close()` to block. This would undo the advice added in commit 8ef0a51,
+      //  but would allow us to merge `close()` and `closeAndWaitForTermination()` and avoid some
+      //  headache.
+      state.closeAndWaitForTermination();
+    }
     boolean shouldShutDownWorkerExecutorInFinally = true;
     try {
       var workerFuture = state.workerFuture;