diff options
author | 2016-08-26 19:54:35 +0000 | |
---|---|---|
committer | 2016-08-27 00:35:56 +0000 | |
commit | 9ab9fefc9199819f7adb305a3d31fd690202f3fa (patch) | |
tree | 2ba459d7647cefc3a5d7c956373d06ed792fe1f3 /src/test/java/com/google/devtools/build | |
parent | ad5a791baa1f482cfd5f46b2f27a20244da16197 (diff) |
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
Diffstat (limited to 'src/test/java/com/google/devtools/build')
-rw-r--r-- | src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java | 48 |
1 files changed, 48 insertions, 0 deletions
diff --git a/src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java b/src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java index fa41f083d5..2c4177994f 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java @@ -18,11 +18,13 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import com.google.common.collect.ImmutableSet; import com.google.common.eventbus.EventBus; import com.google.devtools.build.lib.actions.ResourceManager.ResourceHandle; import com.google.devtools.build.lib.testutil.TestThread; +import com.google.devtools.build.lib.testutil.TestUtils; import java.util.concurrent.CyclicBarrier; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; @@ -274,6 +276,52 @@ public class ResourceManagerTest { } @Test + public void testInterruptedAcquisitionClearsResources() throws Exception { + assertFalse(rm.inUse()); + // Acquire a small amount of resources so that future requests can block (the initial request + // always succeeds even if it's for too much). + TestThread smallThread = + new TestThread() { + @Override + public void runTest() throws InterruptedException { + acquire(1, 0, 0, 0); + } + }; + smallThread.start(); + smallThread.joinAndAssertState(TestUtils.WAIT_TIMEOUT_MILLISECONDS); + TestThread thread1 = + new TestThread() { + @Override + public void runTest() { + Thread.currentThread().interrupt(); + try { + acquire(1999, 0, 0, 0); + fail("Didn't throw interrupted exception"); + } catch (InterruptedException e) { + // Expected. + } + } + }; + thread1.start(); + thread1.joinAndAssertState(TestUtils.WAIT_TIMEOUT_MILLISECONDS); + // This should process the queue. If the request from above is still present, it will take all + // the available memory. But it shouldn't. + rm.setAvailableResources( + ResourceSet.create( + /*memoryMb=*/ 2000.0, /*cpuUsage=*/ 1.0, /*ioUsage=*/ 1.0, /*testCount=*/ 2)); + TestThread thread2 = + new TestThread() { + @Override + public void runTest() throws InterruptedException { + acquire(1999, 0, 0, 0); + release(1999, 0, 0, 0); + } + }; + thread2.start(); + thread2.joinAndAssertState(TestUtils.WAIT_TIMEOUT_MILLISECONDS); + } + + @Test public void testOutOfOrderAllocation() throws Exception { assertFalse(rm.inUse()); TestThread thread1 = new TestThread () { |