Fix resource leakage on interrupt in ResourceManager identified by philwo.

On interrupt, we never released any resources that we had asked to acquire, even though those resources would eventually be acquired.

--
MOS_MIGRATED_REVID=131431321

1 files changed, 21 insertions, 5 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/ResourceManager.java b/src/main/java/com/google/devtools/build/lib/actions/ResourceManager.java
index 216f393c65..c22a1c097f 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ResourceManager.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ResourceManager.java
@@ -23,7 +23,6 @@ import com.google.devtools.build.lib.profiler.AutoProfiler;
 import com.google.devtools.build.lib.profiler.ProfilerTask;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.util.Preconditions;
-
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
@@ -211,6 +210,19 @@ public class ResourceManager {
       if (latch != null) {
         latch.await();
       }
+    } catch (InterruptedException e) {
+      // Synchronize on this to avoid any racing with #processWaitingThreads
+      synchronized (this) {
+        if (latch.getCount() == 0) {
+          // Resources already acquired by other side. Release them, but not inside this
+          // synchronized block to avoid deadlock.
+          release(resources);
+        } else {
+          // Inform other side that resources shouldn't be acquired.
+          latch.countDown();
+        }
+      }
+      throw e;
     } finally {
       threadLocked.set(resources.getCpuUsage() != 0 || resources.getMemoryMb() != 0
           || resources.getIoUsage() != 0 || resources.getLocalTestCount() != 0);
@@ -359,9 +371,14 @@ public class ResourceManager {
     Iterator<Pair<ResourceSet, CountDownLatch>> iterator = requestList.iterator();
     while (iterator.hasNext()) {
       Pair<ResourceSet, CountDownLatch> request = iterator.next();
-      if (areResourcesAvailable(request.first)) {
-        incrementResources(request.first);
-        request.second.countDown();
+      if (request.second.getCount() != 0) {
+        if (areResourcesAvailable(request.first)) {
+          incrementResources(request.first);
+          request.second.countDown();
+          iterator.remove();
+        }
+      } else {
+        // Cancelled by other side.
         iterator.remove();
       }
     }
@@ -405,7 +422,6 @@ public class ResourceManager {
     return cpuIsAvailable && ramIsAvailable && ioIsAvailable && localTestCountIsAvailable;
   }
 
-
   @VisibleForTesting
   synchronized int getWaitCount() {
     return requestList.size();