From 4a3218551c47adfd1d38df3aa6859637edd153f8 Mon Sep 17 00:00:00 2001
From: Armin Braun <me@obrown.io>
Date: Wed, 24 Jul 2019 13:12:57 +0200
Subject: [PATCH] Fix ConnectionManagerTests (#44769) (#44789)

* In both fake connection validators we were potentially executing the listener twice. This lead to the situation that the locking via `connectionLock` that ensures that each listener is only executed once ever
would fail and the lister would run twice (in which case the listeners for that node are already `null` and we get an NPE)
* The fact that two different tests fail is due to the fact that we weren't safely shutting down the threadpool which meant the the task that trips the assertion (on the generic pool) would leak into the next test and fail it
* Closes #44758
---
 .../elasticsearch/transport/ConnectionManagerTests.java    | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/server/src/test/java/org/elasticsearch/transport/ConnectionManagerTests.java b/server/src/test/java/org/elasticsearch/transport/ConnectionManagerTests.java
index d74aa88404d..4677567ab5a 100644
--- a/server/src/test/java/org/elasticsearch/transport/ConnectionManagerTests.java
+++ b/server/src/test/java/org/elasticsearch/transport/ConnectionManagerTests.java
@@ -37,6 +37,7 @@ import java.util.List;
 import java.util.concurrent.BrokenBarrierException;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 
@@ -68,7 +69,7 @@ public class ConnectionManagerTests extends ESTestCase {
 
     @After
     public void stopThreadPool() {
-        threadPool.shutdown();
+        ThreadPool.terminate(threadPool, 10L, TimeUnit.SECONDS);
     }
 
     public void testConnectAndDisconnect() {
@@ -130,7 +131,7 @@ public class ConnectionManagerTests extends ESTestCase {
             ActionListener<Transport.Connection> listener = (ActionListener<Transport.Connection>) invocationOnMock.getArguments()[2];
             if (rarely()) {
                 listener.onResponse(connection);
-            } if (frequently()) {
+            } else if (frequently()) {
                 threadPool.generic().execute(() -> listener.onResponse(connection));
             } else {
                 threadPool.generic().execute(() -> listener.onFailure(new IllegalStateException("dummy exception")));
@@ -143,7 +144,7 @@ public class ConnectionManagerTests extends ESTestCase {
         ConnectionManager.ConnectionValidator validator = (c, p, l) -> {
             if (rarely()) {
                 l.onResponse(null);
-            } if (frequently()) {
+            } else if (frequently()) {
                 threadPool.generic().execute(() -> l.onResponse(null));
             } else {
                 threadPool.generic().execute(() -> l.onFailure(new IllegalStateException("dummy exception")));