Add uncaught exception handler.

dweiss · dweiss · commit 35fab76e63c9 · 2026-03-10T20:52:08.000+01:00
diff --git a/randomizedtesting-jupiter/src/main/java/com/carrotsearch/randomizedtesting/jupiter/DetectThreadLeaksExtension.java b/randomizedtesting-jupiter/src/main/java/com/carrotsearch/randomizedtesting/jupiter/DetectThreadLeaksExtension.java
@@ -1,8 +1,9 @@
 package com.carrotsearch.randomizedtesting.jupiter;
 
-import java.util.Arrays;
+import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Predicate;
@@ -24,6 +25,7 @@ public class DetectThreadLeaksExtension
       ExtensionContext.Namespace.create(DetectThreadLeaksExtension.class);
   private static final String SNAPSHOT_KEY = "snapshot";
   private static final String CONCURRENT_KEY = "concurrent";
+  private static final String UNCAUGHT_EXCEPTION_HANDLER_KEY = "uncaught-exception-handler";
 
   /** Total time budget (ms) to join interrupted threads before giving up. */
   private static final long INTERRUPT_JOIN_MS = 2_000L;
@@ -39,48 +41,67 @@ public void beforeAll(ExtensionContext context) {
       return;
     }
     if (scope(context) == DetectThreadLeaks.Scope.SUITE) {
-      context.getStore(EXTENSION_NAMESPACE).put(SNAPSHOT_KEY, liveThreads(buildFilter(context)));
+      var store = context.getStore(EXTENSION_NAMESPACE);
+      var filter = buildFilter(context);
+      store.put(UNCAUGHT_EXCEPTION_HANDLER_KEY, installUncaughtExceptionHandler());
+      store.put(SNAPSHOT_KEY, liveThreads(filter));
     }
   }
 
   @Override
   public void afterAll(ExtensionContext context) {
-    if (isConcurrentMode(context) || scope(context) != DetectThreadLeaks.Scope.SUITE) {
-      return;
+    if (isConcurrentMode(context) || scope(context) != DetectThreadLeaks.Scope.SUITE) return;
+    var store = context.getStore(EXTENSION_NAMESPACE);
+    var handler = store.get(UNCAUGHT_EXCEPTION_HANDLER_KEY, UncaughtExceptionsHandler.class);
+    try {
+      checkLeaks(
+          store,
+          "suite [" + context.getDisplayName() + "]",
+          linger(context),
+          buildFilter(context),
+          handler);
+    } finally {
+      if (handler != null) handler.restore();
     }
-    checkLeaks(
-        context.getStore(EXTENSION_NAMESPACE),
-        "suite [" + context.getDisplayName() + "]",
-        linger(context),
-        buildFilter(context));
   }
 
   @Override
   public void beforeEach(ExtensionContext context) {
-    if (isConcurrentMode(context) || scope(context) != DetectThreadLeaks.Scope.TEST) {
-      return;
-    }
-    context.getStore(EXTENSION_NAMESPACE).put(SNAPSHOT_KEY, liveThreads(buildFilter(context)));
+    if (isConcurrentMode(context) || scope(context) != DetectThreadLeaks.Scope.TEST) return;
+    var store = context.getStore(EXTENSION_NAMESPACE);
+    var filter = buildFilter(context);
+    store.put(UNCAUGHT_EXCEPTION_HANDLER_KEY, installUncaughtExceptionHandler());
+    store.put(SNAPSHOT_KEY, liveThreads(filter));
   }
 
   @Override
   public void afterEach(ExtensionContext context) {
-    if (isConcurrentMode(context) || scope(context) != DetectThreadLeaks.Scope.TEST) {
-      return;
+    if (isConcurrentMode(context) || scope(context) != DetectThreadLeaks.Scope.TEST) return;
+    var store = context.getStore(EXTENSION_NAMESPACE);
+    var handler = store.get(UNCAUGHT_EXCEPTION_HANDLER_KEY, UncaughtExceptionsHandler.class);
+    try {
+      checkLeaks(
+          store,
+          "test [" + context.getDisplayName() + "]",
+          linger(context),
+          buildFilter(context),
+          handler);
+    } finally {
+      if (handler != null) handler.restore();
     }
-    checkLeaks(
-        context.getStore(EXTENSION_NAMESPACE),
-        "test [" + context.getDisplayName() + "]",
-        linger(context),
-        buildFilter(context));
+  }
+
+  private static UncaughtExceptionsHandler installUncaughtExceptionHandler() {
+    var handler = new UncaughtExceptionsHandler(Thread.getDefaultUncaughtExceptionHandler());
+    Thread.setDefaultUncaughtExceptionHandler(handler);
+    return handler;
   }
 
   private static DetectThreadLeaks.Scope scope(ExtensionContext context) {
     return context.getRequiredTestClass().getAnnotation(DetectThreadLeaks.class).scope();
   }
 
   private static int linger(ExtensionContext context) {
-    // Method-level annotation takes precedence over class-level.
     var methodAnn =
         context
             .getTestMethod()
@@ -94,7 +115,7 @@ private static int linger(ExtensionContext context) {
 
   /**
    * Collects {@link DetectThreadLeaks.ExcludeThreads} filter classes from the entire hierarchy
-   * (method → class → superclasses) and returns a combined predicate that excludes a thread when
+   * (method to class to superclasses) and returns a combined predicate that excludes a thread when
    * any filter matches it.
    */
   private static Predicate<Thread> buildFilter(ExtensionContext context) {
@@ -113,21 +134,19 @@ private static Predicate<Thread> buildFilter(ExtensionContext context) {
     for (Class<?> cls = context.getRequiredTestClass(); cls != null; cls = cls.getSuperclass()) {
       var ann = cls.getAnnotation(DetectThreadLeaks.ExcludeThreads.class);
       if (ann != null) {
-        filterClasses.addAll(Arrays.asList(ann.value()));
+        for (var c : ann.value()) filterClasses.add(c);
       }
     }
 
-    if (filterClasses.isEmpty()) {
-      return t -> false;
-    }
+    if (filterClasses.isEmpty()) return t -> false;
 
     var predicates =
         filterClasses.stream()
             .map(
                 cls -> {
                   try {
                     return (Predicate<Thread>) cls.getDeclaredConstructor().newInstance();
-                  } catch (Exception e) {
+                  } catch (ReflectiveOperationException e) {
                     throw new RuntimeException(
                         "Cannot instantiate thread filter: " + cls.getName(), e);
                   }
@@ -138,7 +157,6 @@ private static Predicate<Thread> buildFilter(ExtensionContext context) {
   }
 
   private static boolean isConcurrentMode(ExtensionContext context) {
-    // Check the concurrent flag stored in beforeAll (class-level context = parent of method ctx).
     return context
         .getParent()
         .map(
@@ -149,53 +167,82 @@ private static boolean isConcurrentMode(ExtensionContext context) {
   }
 
   private static void checkLeaks(
-      ExtensionContext.Store store, String description, int lingerMs, Predicate<Thread> filter) {
+      ExtensionContext.Store store,
+      String description,
+      int lingerMs,
+      Predicate<Thread> filter,
+      UncaughtExceptionsHandler handler) {
     var snapshot = store.get(SNAPSHOT_KEY, HashSet.class);
-    if (snapshot == null) return;
+    AssertionError leakError = null;
 
-    var leaked = leakedSince(snapshot, filter);
-    if (leaked.isEmpty()) return;
+    if (snapshot != null) {
+      var leaked = leakedSince(snapshot, filter);
 
-    // Linger: poll until threads self-terminate or the window expires.
-    if (lingerMs > 0) {
-      long deadline = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(lingerMs);
-      while (!leaked.isEmpty() && System.nanoTime() < deadline) {
-        try {
-          long remainingMs = TimeUnit.NANOSECONDS.toMillis(deadline - System.nanoTime());
-          Thread.sleep(Math.max(1L, Math.min(100L, remainingMs)));
-        } catch (InterruptedException e) {
-          Thread.currentThread().interrupt();
-          break;
+      // Linger: poll until threads self-terminate or the window expires.
+      if (!leaked.isEmpty() && lingerMs > 0) {
+        long deadline = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(lingerMs);
+        while (!leaked.isEmpty() && System.nanoTime() < deadline) {
+          try {
+            long remainingMs = TimeUnit.NANOSECONDS.toMillis(deadline - System.nanoTime());
+            Thread.sleep(Math.max(1L, Math.min(100L, remainingMs)));
+          } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+            break;
+          }
+          leaked = leakedSince(snapshot, filter);
         }
-        leaked = leakedSince(snapshot, filter);
       }
-      if (leaked.isEmpty()) return;
-    }
 
-    // Interrupt leaked threads for cleanup, then wait briefly for them to terminate.
-    leaked.keySet().forEach(Thread::interrupt);
-    long joinDeadline = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(INTERRUPT_JOIN_MS);
-    for (Thread t : leaked.keySet()) {
-      long remaining = TimeUnit.NANOSECONDS.toMillis(joinDeadline - System.nanoTime());
-      if (remaining <= 0) break;
-      try {
-        t.join(remaining);
-      } catch (InterruptedException e) {
-        Thread.currentThread().interrupt();
-        break;
+      if (!leaked.isEmpty()) {
+        // Suppress uncaught exception reporting during the interrupt/join phase to avoid
+        // capturing expected InterruptedException-related exceptions from cleaned-up threads.
+        if (handler != null) handler.stopReporting();
+        try {
+          leaked.keySet().forEach(Thread::interrupt);
+          long joinDeadline = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(INTERRUPT_JOIN_MS);
+          for (Thread t : leaked.keySet()) {
+            long remaining = TimeUnit.NANOSECONDS.toMillis(joinDeadline - System.nanoTime());
+            if (remaining <= 0) break;
+            try {
+              t.join(remaining);
+            } catch (InterruptedException e) {
+              Thread.currentThread().interrupt();
+              break;
+            }
+          }
+        } finally {
+          if (handler != null) handler.resumeReporting();
+        }
+
+        var sb = new StringBuilder(leaked.size() + " thread(s) leaked from " + description + ":");
+        int cnt = 1;
+        for (var entry : leaked.entrySet()) {
+          sb.append(String.format("%n  %2d) %s", cnt++, Threads.threadName(entry.getKey())));
+          for (var ste : entry.getValue()) {
+            sb.append(String.format("%n        at %s", ste));
+          }
+        }
+        leakError = new AssertionError(sb.toString());
       }
     }
 
-    // Report failure with stack traces captured before the interrupt.
-    var sb = new StringBuilder(leaked.size() + " thread(s) leaked from " + description + ":");
-    int cnt = 1;
-    for (var entry : leaked.entrySet()) {
-      sb.append(String.format("%n  %2d) %s", cnt++, Threads.threadName(entry.getKey())));
-      for (var ste : entry.getValue()) {
-        sb.append(String.format("%n        at %s", ste));
-      }
+    // Collect uncaught exceptions regardless of whether threads leaked.
+    List<UncaughtExceptionsHandler.UncaughtException> uncaught =
+        handler != null ? handler.getAndClear() : List.of();
+
+    if (leakError == null && uncaught.isEmpty()) return;
+
+    // Combine: leak error first (if any), uncaught exceptions after; all but the first
+    // are attached as suppressed on the thrown error.
+    var errors = new ArrayList<AssertionError>();
+    if (leakError != null) errors.add(leakError);
+    for (var ue : uncaught) {
+      errors.add(
+          new AssertionError("Uncaught exception in thread [" + ue.threadName() + "]", ue.error()));
     }
-    throw new AssertionError(sb.toString());
+    var first = errors.get(0);
+    errors.subList(1, errors.size()).forEach(first::addSuppressed);
+    throw first;
   }
 
   private static Map<Thread, StackTraceElement[]> leakedSince(
diff --git a/randomizedtesting-jupiter/src/main/java/com/carrotsearch/randomizedtesting/jupiter/UncaughtExceptionsHandler.java b/randomizedtesting-jupiter/src/main/java/com/carrotsearch/randomizedtesting/jupiter/UncaughtExceptionsHandler.java
@@ -0,0 +1,57 @@
+package com.carrotsearch.randomizedtesting.jupiter;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+/** Collects uncaught exceptions from threads during test execution. */
+class UncaughtExceptionsHandler implements Thread.UncaughtExceptionHandler {
+  private static final Logger LOGGER = Logger.getLogger(UncaughtExceptionsHandler.class.getName());
+
+  record UncaughtException(String threadName, Throwable error) {}
+
+  private final Thread.UncaughtExceptionHandler previous;
+  private final List<UncaughtException> exceptions = new ArrayList<>();
+  private boolean reporting = true;
+
+  UncaughtExceptionsHandler(Thread.UncaughtExceptionHandler previous) {
+    this.previous = previous;
+  }
+
+  @Override
+  public void uncaughtException(Thread t, Throwable e) {
+    synchronized (exceptions) {
+      if (reporting) {
+        LOGGER.log(Level.SEVERE, "Uncaught exception in thread: " + Threads.threadName(t), e);
+        exceptions.add(new UncaughtException(Threads.threadName(t), e));
+      }
+    }
+    if (previous != null) previous.uncaughtException(t, e);
+  }
+
+  void stopReporting() {
+    synchronized (exceptions) {
+      reporting = false;
+    }
+  }
+
+  void resumeReporting() {
+    synchronized (exceptions) {
+      reporting = true;
+    }
+  }
+
+  List<UncaughtException> getAndClear() {
+    synchronized (exceptions) {
+      var copy = new ArrayList<>(exceptions);
+      exceptions.clear();
+      return copy;
+    }
+  }
+
+  /** Restores the previous default uncaught exception handler. */
+  void restore() {
+    Thread.setDefaultUncaughtExceptionHandler(previous);
+  }
+}
diff --git a/randomizedtesting-jupiter/src/test/java/com/carrotsearch/randomizedtesting/jupiter/F005_ThreadLeaks.java b/randomizedtesting-jupiter/src/test/java/com/carrotsearch/randomizedtesting/jupiter/F005_ThreadLeaks.java
@@ -343,6 +343,72 @@ private static void startNamedThread(String name) {
     t.start();
   }
 
+  @Nested
+  class TestUncaughtExceptions {
+    @Test
+    void uncaughtExceptionFailsTheTest() {
+      collectExecutionResults(testKitBuilder(UncaughtInTestMethod.class))
+          .results()
+          .allEvents()
+          .finished()
+          .failed()
+          .assertEventsMatchExactly(
+              event(
+                  finishedWithFailure(
+                      instanceOf(AssertionError.class),
+                      new Condition<>(
+                          t ->
+                              t.getCause() instanceof RuntimeException rc
+                                  && "uncaught-test-exception".equals(rc.getMessage()),
+                          "cause is the original RuntimeException"))));
+    }
+
+    @Test
+    void uncaughtExceptionsWithThreadLeaksAreNotReported() {
+      collectExecutionResults(testKitBuilder(UncaughtWithLeak.class))
+          .results()
+          .allEvents()
+          .finished()
+          .failed()
+          .assertEventsMatchExactly(
+              event(
+                  finishedWithFailure(
+                      instanceOf(AssertionError.class),
+                      new Condition<>(t -> t.getCause() == null, "cause is empty."))));
+    }
+
+    @DetectThreadLeaks(scope = DetectThreadLeaks.Scope.SUITE)
+    static class UncaughtInTestMethod extends IgnoreInStandaloneRuns {
+      @Test
+      void testMethod() throws InterruptedException {
+        var t =
+            new Thread(
+                () -> {
+                  throw new RuntimeException("uncaught-test-exception");
+                });
+        t.start();
+        t.join();
+      }
+    }
+
+    @DetectThreadLeaks(scope = DetectThreadLeaks.Scope.TEST)
+    static class UncaughtWithLeak extends IgnoreInStandaloneRuns {
+      @Test
+      void testMethod() {
+        var t1 =
+            new Thread(
+                () -> {
+                  try {
+                    Thread.sleep(TimeUnit.MINUTES.toMillis(1));
+                  } catch (InterruptedException ignored) {
+                    throw new RuntimeException("uncaught-test-exception");
+                  }
+                });
+        t1.start();
+      }
+    }
+  }
+
   /** Starts a daemon thread that sleeps long enough to be observable as a leak. */
   private static void startSleepingThread() {
     var t =
