HADOOP-14851 LambdaTestUtils.eventually() doesn't spin on Assertion failures. Contributed by Steve Loughran

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/LambdaTestUtils.java

@@ -70,7 +70,7 @@ public final class LambdaTestUtils {
      * @throws Exception if the handler wishes to raise an exception
      * that way.
      */
-    Exception evaluate(int timeoutMillis, Exception caught) throws Exception;
+    Throwable evaluate(int timeoutMillis, Throwable caught) throws Throwable;
   }
 
   /**
@@ -116,7 +116,7 @@ public final class LambdaTestUtils {
     Preconditions.checkNotNull(timeoutHandler);
 
     long endTime = Time.now() + timeoutMillis;
-    Exception ex = null;
+    Throwable ex = null;
     boolean running = true;
     int iterations = 0;
     while (running) {
@@ -128,9 +128,11 @@ public final class LambdaTestUtils {
         // the probe failed but did not raise an exception. Reset any
         // exception raised by a previous probe failure.
         ex = null;
-      } catch (InterruptedException | FailFastException e) {
+      } catch (InterruptedException
+          | FailFastException
+          | VirtualMachineError e) {
         throw e;
-      } catch (Exception e) {
+      } catch (Throwable e) {
         LOG.debug("eventually() iteration {}", iterations, e);
         ex = e;
       }
@@ -145,15 +147,20 @@ public final class LambdaTestUtils {
       }
     }
     // timeout
-    Exception evaluate = timeoutHandler.evaluate(timeoutMillis, ex);
-    if (evaluate == null) {
-      // bad timeout handler logic; fall back to GenerateTimeout so the
-      // underlying problem isn't lost.
-      LOG.error("timeout handler {} did not throw an exception ",
-          timeoutHandler);
-      evaluate = new GenerateTimeout().evaluate(timeoutMillis, ex);
+    Throwable evaluate;
+    try {
+      evaluate = timeoutHandler.evaluate(timeoutMillis, ex);
+      if (evaluate == null) {
+        // bad timeout handler logic; fall back to GenerateTimeout so the
+        // underlying problem isn't lost.
+        LOG.error("timeout handler {} did not throw an exception ",
+            timeoutHandler);
+        evaluate = new GenerateTimeout().evaluate(timeoutMillis, ex);
+      }
+    } catch (Throwable throwable) {
+      evaluate = throwable;
     }
-    throw evaluate;
+    return raise(evaluate);
   }
 
   /**
@@ -217,6 +224,7 @@ public final class LambdaTestUtils {
    * @throws Exception the last exception thrown before timeout was triggered
    * @throws FailFastException if raised -without any retry attempt.
    * @throws InterruptedException if interrupted during the sleep operation.
+   * @throws OutOfMemoryError you've run out of memory.
    */
   public static <T> T eventually(int timeoutMillis,
       Callable<T> eval,
@@ -224,7 +232,7 @@ public final class LambdaTestUtils {
     Preconditions.checkArgument(timeoutMillis >= 0,
         "timeoutMillis must be >= 0");
     long endTime = Time.now() + timeoutMillis;
-    Exception ex;
+    Throwable ex;
     boolean running;
     int sleeptime;
     int iterations = 0;
@@ -232,10 +240,12 @@ public final class LambdaTestUtils {
       iterations++;
       try {
         return eval.call();
-      } catch (InterruptedException | FailFastException e) {
+      } catch (InterruptedException
+          | FailFastException
+          | VirtualMachineError e) {
         // these two exceptions trigger an immediate exit
         throw e;
-      } catch (Exception e) {
+      } catch (Throwable e) {
         LOG.debug("evaluate() iteration {}", iterations, e);
         ex = e;
       }
@@ -245,7 +255,26 @@ public final class LambdaTestUtils {
       }
     } while (running);
     // timeout. Throw the last exception raised
-    throw ex;
+    return raise(ex);
+  }
+
+  /**
+   * Take the throwable and raise it as an exception or an error, depending
+   * upon its type. This allows callers to declare that they only throw
+   * Exception (i.e. can be invoked by Callable) yet still rethrow a
+   * previously caught Throwable.
+   * @param throwable Throwable to rethrow
+   * @param <T> expected return type
+   * @return never
+   * @throws Exception if throwable is an Exception
+   * @throws Error if throwable is not an Exception
+   */
+  private static <T> T raise(Throwable throwable) throws Exception {
+    if (throwable instanceof Exception) {
+      throw (Exception) throwable;
+    } else {
+      throw (Error) throwable;
+    }
   }
 
   /**
@@ -365,6 +394,7 @@ public final class LambdaTestUtils {
    * @throws Exception any other exception raised
    * @throws AssertionError if the evaluation call didn't raise an exception.
    */
+  @SuppressWarnings("unchecked")
   public static <E extends Throwable> E intercept(
       Class<E> clazz,
       VoidCallable eval)
@@ -487,14 +517,14 @@ public final class LambdaTestUtils {
      * @return TimeoutException
      */
     @Override
-    public Exception evaluate(int timeoutMillis, Exception caught)
-        throws Exception {
+    public Throwable evaluate(int timeoutMillis, Throwable caught)
+        throws Throwable {
       String s = String.format("%s: after %d millis", message,
           timeoutMillis);
       String caughtText = caught != null
           ? ("; " + robustToString(caught)) : "";
 
-      return (TimeoutException) (new TimeoutException(s + caughtText)
+      return (new TimeoutException(s + caughtText)
                                      .initCause(caught));
     }
   }

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/TestLambdaTestUtils.java

@@ -25,6 +25,7 @@ import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.util.concurrent.Callable;
 import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import static org.apache.hadoop.test.LambdaTestUtils.*;
 import static org.apache.hadoop.test.GenericTestUtils.*;
@@ -123,6 +124,27 @@ public class TestLambdaTestUtils extends Assert {
         minCount <= retry.getInvocationCount());
   }
 
+  /**
+   * Raise an exception.
+   * @param e exception to raise
+   * @return never
+   * @throws Exception passed in exception
+   */
+  private boolean r(Exception e) throws Exception {
+    throw e;
+  }
+
+  /**
+   * Raise an error.
+   * @param e error to raise
+   * @return never
+   * @throws Exception never
+   * @throws Error the passed in error
+   */
+  private boolean r(Error e) throws Exception {
+    throw e;
+  }
+
   @Test
   public void testAwaitAlwaysTrue() throws Throwable {
     await(TIMEOUT,
@@ -140,7 +162,7 @@ public class TestLambdaTestUtils extends Assert {
           TIMEOUT_FAILURE_HANDLER);
       fail("should not have got here");
     } catch (TimeoutException e) {
-      assertTrue(retry.getInvocationCount() > 4);
+      assertMinRetryCount(1);
     }
   }
 
@@ -316,9 +338,7 @@ public class TestLambdaTestUtils extends Assert {
     IOException ioe = intercept(IOException.class,
         () -> await(
             TIMEOUT,
-            () -> {
-              throw new IOException("inner " + ++count);
-            },
+            () -> r(new IOException("inner " + ++count)),
             retry,
             (timeout, ex) -> ex));
     assertRetryCount(count - 1);
@@ -339,9 +359,7 @@ public class TestLambdaTestUtils extends Assert {
   public void testInterceptAwaitFailFastLambda() throws Throwable {
     intercept(FailFastException.class,
         () -> await(TIMEOUT,
-            () -> {
-              throw new FailFastException("ffe");
-            },
+            () -> r(new FailFastException("ffe")),
             retry,
             (timeout, ex) -> ex));
     assertRetryCount(0);
@@ -361,14 +379,13 @@ public class TestLambdaTestUtils extends Assert {
     assertRetryCount(0);
   }
 
+
   @Test
   public void testInterceptEventuallyLambdaFailures() throws Throwable {
     intercept(IOException.class,
         "oops",
         () -> eventually(TIMEOUT,
-            () -> {
-              throw new IOException("oops");
-            },
+            () -> r(new IOException("oops")),
             retry));
     assertMinRetryCount(1);
   }
@@ -385,11 +402,95 @@ public class TestLambdaTestUtils extends Assert {
     intercept(FailFastException.class, "oops",
         () -> eventually(
             TIMEOUT,
-            () -> {
-              throw new FailFastException("oops");
-            },
+            () -> r(new FailFastException("oops")),
             retry));
     assertRetryCount(0);
   }
 
+  /**
+   * Verify that assertions trigger catch and retry.
+   * @throws Throwable if the code is broken
+   */
+  @Test
+  public void testEventuallySpinsOnAssertions() throws Throwable {
+    AtomicInteger counter = new AtomicInteger(0);
+    eventually(TIMEOUT,
+        () -> {
+          while (counter.incrementAndGet() < 5) {
+            fail("if you see this, we are in trouble");
+          }
+        },
+        retry);
+    assertMinRetryCount(4);
+  }
+
+  /**
+   * Verify that VirtualMachineError errors are immediately rethrown.
+   * @throws Throwable if the code is broken
+   */
+  @Test
+  public void testInterceptEventuallyThrowsVMErrors() throws Throwable {
+    intercept(OutOfMemoryError.class, "OOM",
+        () -> eventually(
+            TIMEOUT,
+            () -> r(new OutOfMemoryError("OOM")),
+            retry));
+    assertRetryCount(0);
+  }
+
+  /**
+   * Verify that you can declare that an intercept will intercept Errors.
+   * @throws Throwable if the code is broken
+   */
+  @Test
+  public void testInterceptHandlesErrors() throws Throwable {
+    intercept(OutOfMemoryError.class, "OOM",
+        () -> r(new OutOfMemoryError("OOM")));
+  }
+
+  /**
+   * Verify that if an Error raised is not the one being intercepted,
+   * it gets rethrown.
+   * @throws Throwable if the code is broken
+   */
+  @Test
+  public void testInterceptRethrowsVMErrors() throws Throwable {
+    intercept(StackOverflowError.class, "",
+        () -> intercept(OutOfMemoryError.class, "",
+            () -> r(new StackOverflowError())));
+  }
+
+  @Test
+  public void testAwaitHandlesAssertions() throws Throwable {
+    // await a state which is never reached, expect a timeout exception
+    // with the text "failure" in it
+    TimeoutException ex = intercept(TimeoutException.class,
+        "failure",
+        () -> await(TIMEOUT,
+            () -> r(new AssertionError("failure")),
+            retry,
+            TIMEOUT_FAILURE_HANDLER));
+
+    // the retry handler must have been invoked
+    assertMinRetryCount(1);
+    // and the nested cause is tha raised assertion
+    if (!(ex.getCause() instanceof AssertionError)) {
+      throw ex;
+    }
+  }
+
+  @Test
+  public void testAwaitRethrowsVMErrors() throws Throwable {
+    // await a state which is never reached, expect a timeout exception
+    // with the text "failure" in it
+    intercept(StackOverflowError.class,
+        () -> await(TIMEOUT,
+            () -> r(new StackOverflowError()),
+            retry,
+            TIMEOUT_FAILURE_HANDLER));
+
+    // the retry handler must not have been invoked
+    assertMinRetryCount(0);
+  }
+
 }