Bladeren bron

Move assertion for open channels under TcpTransport lock

TcpTransport has an actual mechanism to stop resources in subclasses.
Instead of overriding `doStop` subclasses should override `stopInternal`
that is executed under the connection lock guaranteeing that there is no
concurrency etc.

Relates to #22554
Simon Willnauer 8 jaren geleden
bovenliggende
commit
8a0393f718

+ 1 - 1
core/src/main/java/org/elasticsearch/transport/TcpTransport.java

@@ -840,7 +840,7 @@ public abstract class TcpTransport<Channel> extends AbstractLifecycleComponent i
     }
 
     @Override
-    protected void doClose() {
+    protected final void doClose() {
     }
 
     @Override

+ 3 - 12
test/framework/src/main/java/org/elasticsearch/transport/MockTcpTransport.java

@@ -400,23 +400,14 @@ public class MockTcpTransport extends TcpTransport<MockTcpTransport.MockChannel>
     @Override
     protected void stopInternal() {
         ThreadPool.terminate(executor, 10, TimeUnit.SECONDS);
+        synchronized (openChannels) {
+            assert openChannels.isEmpty() : "there are still open channels: " + openChannels;
+        }
     }
 
     @Override
     protected Version getCurrentVersion() {
         return mockVersion;
     }
-
-    @Override
-    protected void doClose() {
-        if (Thread.currentThread().isInterrupted() == false) {
-            // TCPTransport might be interrupted due to a timeout waiting for connections to be closed.
-            // in this case the thread is interrupted and we can't tell if we really missed something or if we are
-            // still closing connections. in such a case we don't assert the open channels
-            synchronized (openChannels) {
-                assert openChannels.isEmpty() : "there are still open channels: " + openChannels;
-            }
-        }
-    }
 }