8221892: ThreadPoolExecutor: Thread.isAlive() is not equivalent to not being startable

Reviewed-by: martin, dholmes

src/java.base/share/classes/java/util/concurrent/ThreadPoolExecutor.java

@@ -922,13 +922,13 @@ public class ThreadPoolExecutor extends AbstractExecutorService {
 
                     if (isRunning(c) ||
                         (runStateLessThan(c, STOP) && firstTask == null)) {
-                        if (t.isAlive()) // precheck that t is startable
+                        if (t.getState() != Thread.State.NEW)
                             throw new IllegalThreadStateException();
                         workers.add(w);
+                        workerAdded = true;
                         int s = workers.size();
                         if (s > largestPoolSize)
                             largestPoolSize = s;
-                        workerAdded = true;
                     }
                 } finally {
                     mainLock.unlock();

test/jdk/java/util/concurrent/tck/ThreadPoolExecutorTest.java

@@ -2011,4 +2011,49 @@ public class ThreadPoolExecutorTest extends JSR166TestCase {
         assertTrue(p.getQueue().isEmpty());
     }
 
+    public void testThreadFactoryReturnsTerminatedThread_shouldThrow() {
+        if (!testImplementationDetails)
+            return;
+
+        ThreadFactory returnsTerminatedThread = runnableIgnored -> {
+            Thread thread = new Thread(() -> {});
+            thread.start();
+            try { thread.join(); }
+            catch (InterruptedException ex) { throw new Error(ex); }
+            return thread;
+        };
+        ThreadPoolExecutor p =
+            new ThreadPoolExecutor(1, 1, 1, SECONDS,
+                                   new ArrayBlockingQueue<Runnable>(1),
+                                   returnsTerminatedThread);
+        try (PoolCleaner cleaner = cleaner(p)) {
+            assertThrows(IllegalThreadStateException.class,
+                         () -> p.execute(() -> {}));
+        }
+    }
+
+    public void testThreadFactoryReturnsStartedThread_shouldThrow() {
+        if (!testImplementationDetails)
+            return;
+
+        CountDownLatch latch = new CountDownLatch(1);
+        Runnable awaitLatch = () -> {
+            try { latch.await(); }
+            catch (InterruptedException ex) { throw new Error(ex); }};
+        ThreadFactory returnsStartedThread = runnable -> {
+            Thread thread = new Thread(awaitLatch);
+            thread.start();
+            return thread;
+        };
+        ThreadPoolExecutor p =
+            new ThreadPoolExecutor(1, 1, 1, SECONDS,
+                                   new ArrayBlockingQueue<Runnable>(1),
+                                   returnsStartedThread);
+        try (PoolCleaner cleaner = cleaner(p)) {
+            assertThrows(IllegalThreadStateException.class,
+                         () -> p.execute(() -> {}));
+            latch.countDown();
+        }
+    }
+
 }