Cancel execution when run is interrupted.

Bug: 123529934
Test: unit tests
Bug: 117420403
Change-Id: Ibded861d000d84f54bb10433c8cf7815d5d83b70
Merged-In: Ibded861d000d84f54bb10433c8cf7815d5d83b70
diff --git a/src/com/android/tradefed/util/RunUtil.java b/src/com/android/tradefed/util/RunUtil.java
index 923ebd5..40d3bcc 100644
--- a/src/com/android/tradefed/util/RunUtil.java
+++ b/src/com/android/tradefed/util/RunUtil.java
@@ -326,39 +326,45 @@
                 CLog.d("Running command without timeout.");
             }
         }
-        runThread.start();
-        long startTime = System.currentTimeMillis();
-        long pollIterval = 0;
-        if (timeout > 0l && timeout < THREAD_JOIN_POLL_INTERVAL) {
-            // only set the pollInterval if we have a timeout
-            pollIterval = timeout;
-        } else {
-            pollIterval = THREAD_JOIN_POLL_INTERVAL;
-        }
-        do {
-            try {
-                runThread.join(pollIterval);
-            } catch (InterruptedException e) {
-                if (isInterruptAllowed()) {
-                    CLog.i("runTimed: interrupted while joining the runnable");
-                    break;
+
+        try {
+            runThread.start();
+            long startTime = System.currentTimeMillis();
+            long pollInterval = 0;
+            if (timeout > 0L && timeout < THREAD_JOIN_POLL_INTERVAL) {
+                // only set the pollInterval if we have a timeout
+                pollInterval = timeout;
+            } else {
+                pollInterval = THREAD_JOIN_POLL_INTERVAL;
+            }
+            do {
+                try {
+                    runThread.join(pollInterval);
+                } catch (InterruptedException e) {
+                    if (isInterruptAllowed()) {
+                        CLog.i("runTimed: interrupted while joining the runnable");
+                        break;
+                    } else {
+                        CLog.i("runTimed: currently uninterruptible, ignoring interrupt");
+                    }
                 }
-                else {
-                    CLog.i("runTimed: received an interrupt but uninterruptible mode, ignoring");
-                }
+                mInterrupter.checkInterrupted();
+            } while ((timeout == 0L || (System.currentTimeMillis() - startTime) < timeout)
+                    && runThread.isAlive());
+            // Snapshot the status when out of the run loop because thread may terminate and return
+            // a false FAILED instead of TIMED_OUT.
+            CommandStatus status = runThread.getStatus();
+            if (CommandStatus.TIMED_OUT.equals(status) || CommandStatus.EXCEPTION.equals(status)) {
+                CLog.i("runTimed: Calling interrupt, status is %s", status);
+                runThread.cancel();
             }
             mInterrupter.checkInterrupted();
-        } while ((timeout == 0l || (System.currentTimeMillis() - startTime) < timeout)
-                && runThread.isAlive());
-        // Snapshot the status when out of the run loop because thread may terminate and return a
-        // false FAILED instead of TIMED_OUT.
-        CommandStatus status = runThread.getStatus();
-        if (CommandStatus.TIMED_OUT.equals(status) || CommandStatus.EXCEPTION.equals(status)) {
-            CLog.i("runTimed: Calling interrupt, status is %s", status);
+            return status;
+
+        } finally {
+            // always ensure the execution is cancelled
             runThread.cancel();
         }
-        mInterrupter.checkInterrupted();
-        return status;
     }
 
     /**
diff --git a/tests/src/com/android/tradefed/util/RunUtilTest.java b/tests/src/com/android/tradefed/util/RunUtilTest.java
index b5a67b1..0ef6bf1 100644
--- a/tests/src/com/android/tradefed/util/RunUtilTest.java
+++ b/tests/src/com/android/tradefed/util/RunUtilTest.java
@@ -15,6 +15,7 @@
  */
 package com.android.tradefed.util;
 
+import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.doThrow;
 
@@ -106,6 +107,7 @@
         IRunUtil.IRunnableResult mockRunnable = EasyMock.createStrictMock(
                 IRunUtil.IRunnableResult.class);
         EasyMock.expect(mockRunnable.run()).andReturn(Boolean.TRUE);
+        mockRunnable.cancel(); // always ensure execution is cancelled
         EasyMock.replay(mockRunnable);
         assertEquals(CommandStatus.SUCCESS,
                 mRunUtil.runTimed(SHORT_TIMEOUT_MS, mockRunnable, true));
@@ -118,6 +120,7 @@
         IRunUtil.IRunnableResult mockRunnable = EasyMock.createStrictMock(
                 IRunUtil.IRunnableResult.class);
         EasyMock.expect(mockRunnable.run()).andReturn(Boolean.FALSE);
+        mockRunnable.cancel(); // always ensure execution is cancelled
         EasyMock.replay(mockRunnable);
         assertEquals(CommandStatus.FAILED,
                 mRunUtil.runTimed(SHORT_TIMEOUT_MS, mockRunnable, true));
@@ -130,15 +133,32 @@
         IRunUtil.IRunnableResult mockRunnable = EasyMock.createStrictMock(
                 IRunUtil.IRunnableResult.class);
         EasyMock.expect(mockRunnable.run()).andThrow(new RuntimeException());
-        mockRunnable.cancel();
+        mockRunnable.cancel(); // cancel due to exception
+        mockRunnable.cancel(); // always ensure execution is cancelled
         EasyMock.replay(mockRunnable);
         assertEquals(CommandStatus.EXCEPTION,
                 mRunUtil.runTimed(SHORT_TIMEOUT_MS, mockRunnable, true));
     }
 
-    /**
-     * Test that {@link RunUtil#runTimedCmd(long, String[])} fails when given a garbage command.
-     */
+    /** Test interrupted case for {@link RunUtil#runTimed(long, IRunnableResult, boolean)}. */
+    public void testRunTimed_interrupted() {
+        IRunnableResult runnable = Mockito.mock(IRunnableResult.class);
+        CommandInterrupter interrupter = Mockito.mock(CommandInterrupter.class);
+        RunUtil runUtil = new RunUtil(interrupter);
+
+        // interrupted during execution
+        doNothing().doThrow(RunInterruptedException.class).when(interrupter).checkInterrupted();
+
+        try {
+            runUtil.runTimed(VERY_SHORT_TIMEOUT_MS, runnable, true);
+            fail("RunInterruptedException was expected, but not thrown.");
+        } catch (RunInterruptedException e) {
+            // execution was cancelled due to interruption
+            Mockito.verify(runnable, Mockito.times(1)).cancel();
+        }
+    }
+
+    /** Test that {@link RunUtil#runTimedCmd(long, String[])} fails when given a garbage command. */
     public void testRunTimedCmd_failed() {
         RunUtil spyUtil = new SpyRunUtil(true);
         CommandResult result = spyUtil.runTimedCmd(1000, "blahggggwarggg");