[7.4.0] Do no crash if a requested resource is not available

src/main/java/com/google/devtools/build/lib/actions/BUILD

@@ -488,6 +488,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/worker",
         "//src/main/java/com/google/devtools/build/lib/worker:worker_key",
         "//src/main/java/com/google/devtools/build/lib/worker:worker_pool",
+        "//src/main/protobuf:failure_details_java_proto",
         "//third_party:guava",
         "//third_party:jsr305",
     ],

src/main/java/com/google/devtools/build/lib/actions/ResourceManager.java

@@ -22,6 +22,7 @@
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
 import com.google.devtools.build.lib.profiler.AutoProfiler;
 import com.google.devtools.build.lib.profiler.ProfilerTask;
+import com.google.devtools.build.lib.server.FailureDetails;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.worker.Worker;
 import com.google.devtools.build.lib.worker.WorkerKey;
@@ -236,14 +237,17 @@ public void setWorkerPool(WorkerPool workerPool) {
    */
   public ResourceHandle acquireResources(
       ActionExecutionMetadata owner, ResourceSet resources, ResourcePriority priority)
-      throws InterruptedException, IOException {
+      throws InterruptedException, IOException, ExecException {
     Preconditions.checkNotNull(
         resources, "acquireResources called with resources == NULL during %s", owner);
     Preconditions.checkState(
         !threadHasResources(), "acquireResources with existing resource lock during %s", owner);
 
     LatchWithWorker latchWithWorker = null;
 
+    // Validate requested resources exist before attempting to acquire.
+    assertResourcesTracked(resources);
+
     AutoProfiler p =
         profiled("Acquiring resources for: " + owner.describe(), ProfilerTask.ACTION_LOCK);
     try {
@@ -468,12 +472,24 @@ private synchronized void processWaitingThreads(Deque<Pair<ResourceSet, LatchWit
   }
 
   /** Throws an exception if requested extra resource isn't being tracked */
-  private void assertResourcesTracked(ResourceSet resources) throws NoSuchElementException {
+  private void assertResourcesTracked(ResourceSet resources) throws ExecException {
     for (Map.Entry<String, Double> resource : resources.getResources().entrySet()) {
       String key = resource.getKey();
       if (!availableResources.getResources().containsKey(key)) {
-        throw new NoSuchElementException(
-            "Resource " + key + " is not tracked in this resource set.");
+        StringBuilder message = new StringBuilder();
+        message.append("Resource ");
+        message.append(key);
+        message.append(" is not being tracked by the resource manager.");
+        message.append(" Available resources are: ");
+        message.append(String.join(", ", availableResources.getResources().keySet()));
+        throw new UserExecException(
+            FailureDetails.FailureDetail.newBuilder()
+                .setMessage(message.toString())
+                .setLocalExecution(
+                    FailureDetails.LocalExecution.newBuilder()
+                        .setCode(FailureDetails.LocalExecution.Code.UNTRACKED_RESOURCE)
+                        .build())
+                .build());
       }
     }
   }
@@ -507,10 +523,6 @@ boolean areResourcesAvailable(ResourceSet resources) throws NoSuchElementExcepti
       }
     }
 
-    // We test for tracking of extra resources whenever acquired and throw an
-    // exception before acquiring any untracked resource.
-    assertResourcesTracked(resources);
-
     if (usedResources.isEmpty() && usedLocalTestCount == 0) {
       return true;
     }

src/main/protobuf/failure_details.proto

@@ -718,6 +718,7 @@ message LocalExecution {
   enum Code {
     LOCAL_EXECUTION_UNKNOWN = 0 [(metadata) = { exit_code: 37 }];
     LOCKFREE_OUTPUT_PREREQ_UNMET = 1 [(metadata) = { exit_code: 2 }];
+    UNTRACKED_RESOURCE = 2 [(metadata) = { exit_code: 1 }];
   }
 
   Code code = 1;

src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java

@@ -99,17 +99,17 @@ public boolean validateObject(WorkerKey key, PooledObject<Worker> p) {
   }
 
   private ResourceHandle acquire(double ram, double cpu, int tests, ResourcePriority priority)
-      throws InterruptedException, IOException {
+      throws InterruptedException, IOException, ExecException {
     return rm.acquireResources(resourceOwner, ResourceSet.create(ram, cpu, tests), priority);
   }
 
   private ResourceHandle acquire(double ram, double cpu, int tests)
-      throws InterruptedException, IOException {
+      throws InterruptedException, IOException, ExecException {
     return acquire(ram, cpu, tests, ResourcePriority.LOCAL);
   }
 
   private ResourceHandle acquire(double ram, double cpu, int tests, String mnemonic)
-      throws InterruptedException, IOException {
+      throws InterruptedException, IOException, ExecException {
 
     return rm.acquireResources(
         resourceOwner,
@@ -127,7 +127,7 @@ private ResourceHandle acquire(
       ImmutableMap<String, Double> extraResources,
       int tests,
       ResourcePriority priority)
-      throws InterruptedException, IOException, NoSuchElementException {
+      throws InterruptedException, IOException, NoSuchElementException, ExecException {
     ImmutableMap.Builder<String, Double> resources = ImmutableMap.builder();
     resources.putAll(extraResources).put(ResourceSet.MEMORY, ram).put(ResourceSet.CPU, cpu);
     return rm.acquireResources(
@@ -137,7 +137,7 @@ private ResourceHandle acquire(
   @CanIgnoreReturnValue
   private ResourceHandle acquire(
       double ram, double cpu, ImmutableMap<String, Double> extraResources, int tests)
-      throws InterruptedException, IOException, NoSuchElementException {
+      throws InterruptedException, IOException, NoSuchElementException, ExecException {
     return acquire(ram, cpu, extraResources, tests, ResourcePriority.LOCAL);
   }
 
@@ -676,7 +676,7 @@ public void testNonexistingResource() throws Exception {
         new TestThread(
             () ->
                 assertThrows(
-                    NoSuchElementException.class,
+                    UserExecException.class,
                     () -> acquire(0, 0, ImmutableMap.of("nonexisting", 1.0), 0)));
     thread1.start();
     thread1.joinAndAssertState(1000);