ThreadGroup instance leaked when using Timeout rule (#1517)

junit-team · Jun 20, 2018 · 63fd277 · 63fd277
1 parent 54b338c
commit 63fd277
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 6 deletions.
diff --git a/src/main/java/org/junit/internal/runners/statements/FailOnTimeout.java b/src/main/java/org/junit/internal/runners/statements/FailOnTimeout.java
@@ -122,12 +122,26 @@ public void evaluate() throws Throwable {
         FutureTask<Throwable> task = new FutureTask<Throwable>(callable);
         ThreadGroup threadGroup = new ThreadGroup("FailOnTimeoutGroup");
         Thread thread = new Thread(threadGroup, task, "Time-limited test");
-        thread.setDaemon(true);
-        thread.start();
-        callable.awaitStarted();
-        Throwable throwable = getResult(task, thread);
-        if (throwable != null) {
-            throw throwable;
+        try {
+            thread.setDaemon(true);
+            thread.start();
+            callable.awaitStarted();
+            Throwable throwable = getResult(task, thread);
+            if (throwable != null) {
+                throw throwable;
+            }
+        } finally {
+            try {
+                thread.join(1);
+            } catch (InterruptedException e) {
+                Thread.currentThread().interrupt();
+            }
+            try {
+                threadGroup.destroy();
+            } catch (IllegalThreadStateException e) {
+                // If a thread from the group is still alive, the ThreadGroup cannot be destroyed.
+                // Swallow the exception to keep the same behavior prior to this change.
+            }
         }
     }
 

diff --git a/src/test/java/org/junit/internal/runners/statements/FailOnTimeoutTest.java b/src/test/java/org/junit/internal/runners/statements/FailOnTimeoutTest.java
@@ -3,6 +3,7 @@
 import static java.lang.Long.MAX_VALUE;
 import static java.lang.Math.atan;
 import static java.lang.System.currentTimeMillis;
+import static java.lang.Thread.currentThread;
 import static java.lang.Thread.sleep;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static org.junit.Assert.assertEquals;
@@ -13,6 +14,10 @@
 import static org.junit.Assert.fail;
 import static org.junit.internal.runners.statements.FailOnTimeout.builder;
 
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
 import org.junit.Test;
@@ -202,4 +207,23 @@ private void notTheRealCauseOfTheTimeout() {
             }
         }
     }
+
+    @Test
+    public void threadGroupNotLeaked() throws Throwable {
+        Collection<ThreadGroup> groupsBeforeSet = subGroupsOfCurrentThread();
+
+        evaluateWithWaitDuration(0);
+
+        for (ThreadGroup group: subGroupsOfCurrentThread()) {
+            if (!groupsBeforeSet.contains(group) && "FailOnTimeoutGroup".equals(group.getName())) {
+                fail("A 'FailOnTimeoutGroup' thread group remains referenced after the test execution.");
+            }
+        }
+    }
+
+    private Collection<ThreadGroup> subGroupsOfCurrentThread() {
+        ThreadGroup[] subGroups = new ThreadGroup[256];
+        int numGroups = currentThread().getThreadGroup().enumerate(subGroups);
+        return Arrays.asList(subGroups).subList(0, numGroups);
+    }
 }