8195716: BootstrapLoggerTest : Executor still alive
authordfuchs
Fri, 01 Feb 2019 14:15:49 +0000
changeset 53603 c53a3355dbb4
parent 53602 d1282cd2c1fc
child 53604 635361ec5491
8195716: BootstrapLoggerTest : Executor still alive Summary: Make sure the test joins the active executor thread before waiting for the executor to be GC'ed. Reviewed-by: mchung, martin
test/jdk/java/lang/System/LoggerFinder/internal/BootstrapLogger/BootstrapLoggerTest.java
--- a/test/jdk/java/lang/System/LoggerFinder/internal/BootstrapLogger/BootstrapLoggerTest.java	Fri Feb 01 21:43:37 2019 +0900
+++ b/test/jdk/java/lang/System/LoggerFinder/internal/BootstrapLogger/BootstrapLoggerTest.java	Fri Feb 01 14:15:49 2019 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -22,6 +22,7 @@
  */
 
 import java.io.PrintStream;
+import java.lang.ref.Reference;
 import java.lang.ref.ReferenceQueue;
 import java.lang.ref.WeakReference;
 import java.lang.reflect.Array;
@@ -37,6 +38,7 @@
 import java.security.Policy;
 import java.security.ProtectionDomain;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -156,7 +158,7 @@
                     .collect(Collectors.toSet());
             set.stream().forEach(t -> LogStream.err.println("Found: " + t));
             if (set.size() > 1) {
-                throw new RuntimeException("Too many bootsrap threads found");
+                throw new RuntimeException("Too many bootstrap threads found");
             }
             Optional<Thread> t = set.stream().findFirst();
             if (t.isPresent()) {
@@ -292,46 +294,73 @@
         // of the executor.
         SimplePolicy.allowAll.set(Boolean.TRUE);
         try {
-            Stream<Thread> stream = Thread.getAllStackTraces().keySet().stream();
-            stream.filter((t) -> t.getName().startsWith("BootstrapMessageLoggerTask-"))
-                    .forEach(t -> LogStream.err.println(t));
+            // Though unlikely, it is not impossible that the bootstrap logger
+            // executor may have released its first thread and spawned a new one.
+            // If that happened then the executor itself might have been GC'ed
+            // as well and a new one might have been created.
+            // The code below will lookup the executor threads again and
+            // join them.
+            // Only one may be active at a given time, but that might not
+            // be the one referenced by threadRef.
+            // We're just making sure all of them have stopped running
+            // before verifying that the executor is eventually GC'ed.
+            final WeakReference<Thread> previous = threadRef;
+            Stream<WeakReference<Thread>> stream = Thread.getAllStackTraces().keySet().stream()
+               .filter((t) -> t.getName().startsWith("BootstrapMessageLoggerTask-"))
+               .filter((t) -> previous == null ? true : t != previous.get())
+               .map((t) -> new WeakReference<>(t, queue));
+            List<WeakReference<Thread>> threads = stream.collect(Collectors.toList());
+            if (previous != null) threads.add(previous);
+            threads.forEach(t -> LogStream.err.println(t.get()));
             stream = null;
-            if (threadRef != null && test == TestCase.SECURE_AND_WAIT) {
-                Thread t = threadRef.get();
-                if (t != null) {
-                    if (!(Boolean)isAlive.invoke(null)) {
-                        throw new RuntimeException("Executor already terminated");
+
+            if (test == TestCase.SECURE_AND_WAIT) {
+                // First wait for all executor threads to terminate
+                for (var ref : threads) {
+                    Thread t = ref.get();
+                    if (t != null) {
+                        if (!(Boolean)isAlive.invoke(null) && t.isAlive()) {
+                            throw new RuntimeException("Executor already terminated");
+                        } else {
+                            LogStream.err.println("Executor still alive as expected: " + t.getName());
+                        }
+                        LogStream.err.println("Waiting for " + t.getName() + " to terminate (join)");
+                        t.join(60_000);
+                        t = null;
                     } else {
-                        LogStream.err.println("Executor still alive as expected.");
+                        LogStream.err.println("WeakReference<Thread> is already cleared.");
+                        long count = Thread.getAllStackTraces().keySet().stream()
+                            .filter((tr) -> tr.getName().startsWith("BootstrapMessageLoggerTask-"))
+                            .count();
+                        if (count != 0) {
+                            LogStream.err.println("There are " + count + " threads still lingering.");
+                        }
                     }
-                    LogStream.err.println("Waiting for " + t.getName() + " to terminate (join)");
-                    t.join(60_000);
-                    t = null;
                 }
-                LogStream.err.println("Calling System.gc()");
-                System.gc();
-                LogStream.err.println("Waiting for BootstrapMessageLoggerTask to be gc'ed");
-                while (queue.remove(1000) == null) {
+                // Then wait until all the executor threads are GC'ed
+                while (!threads.isEmpty()) {
                     LogStream.err.println("Calling System.gc()");
                     System.gc();
-                }
+                    LogStream.err.println("Waiting for BootstrapMessageLoggerTask to be gc'ed");
+                    Reference<?> tref;
+                    while ((tref = queue.remove(1000)) == null) {
+                        LogStream.err.println("Calling System.gc()");
+                        System.gc();
+                    }
 
-                // Call the reference here to make sure threadRef will not be
-                // eagerly garbage collected before the thread it references.
-                // otherwise, it might not be enqueued, resulting in the
-                // queue.remove() call above to always return null....
-                if (threadRef.get() != null) {
-                    throw new RuntimeException("Reference should have been cleared");
+                    threads.remove(tref);
+                    LogStream.err.println("BootstrapMessageLoggerTask has been gc'ed: "
+                                          + threads.size() + " remaining...");
                 }
-
-                LogStream.err.println("BootstrapMessageLoggerTask has been gc'ed");
-                // Wait for the executor to be gc'ed...
+                // Then wait for the executor to be gc'ed...
+                LogStream.err.println("Waiting for the executor to be gc'ed: Calling System.gc()");
+                System.gc();
                 for (int i=0; i<10; i++) {
-                    LogStream.err.println("Calling System.gc()");
-                    System.gc();
                     if (!(Boolean)isAlive.invoke(null)) break;
                     // It would be unexpected that we reach here...
                     Thread.sleep(1000);
+                    LogStream.err.println("Calling System.gc()");
+                    System.gc();
                 }
 
                 if ((Boolean)isAlive.invoke(null)) {