Fix NIO concurrency issue that removes connections from the poller.

This is the source of the intermittent WebSocket test failure so this
commit also removes the associated debug code for that issue.

java/org/apache/tomcat/util/net/NioEndpoint.java

@@ -1522,7 +1522,14 @@ public SocketProcessor(SocketWrapperBase<NioChannel> socketWrapper, SocketEvent
 
         @Override
         protected void doRun() {
-            NioChannel socket = socketWrapper.getSocket();
+            /*
+             * Do not cache and re-use the value of socketWrapper.getSocket() in
+             * this method. If the socket closes the value will be updated to
+             * CLOSED_NIO_CHANNEL and the previous value potentially re-used for
+             * a new connection. That can result in a stale cached value which
+             * in turn can result in unintentionally closing currently active
+             * connections.
+             */
             Poller poller = NioEndpoint.this.poller;
             if (poller == null) {
                 socketWrapper.close();
@@ -1532,7 +1539,7 @@ protected void doRun() {
             try {
                 int handshake = -1;
                 try {
-                    if (socket.isHandshakeComplete()) {
+                    if (socketWrapper.getSocket().isHandshakeComplete()) {
                         // No TLS handshaking required. Let the handler
                         // process this socket / event combination.
                         handshake = 0;
@@ -1542,7 +1549,7 @@ protected void doRun() {
                         // if the handshake failed.
                         handshake = -1;
                     } else {
-                        handshake = socket.handshake(event == SocketEvent.OPEN_READ, event == SocketEvent.OPEN_WRITE);
+                        handshake = socketWrapper.getSocket().handshake(event == SocketEvent.OPEN_READ, event == SocketEvent.OPEN_WRITE);
                         // The handshake process reads/writes from/to the
                         // socket. status may therefore be OPEN_WRITE once
                         // the handshake completes. However, the handshake
@@ -1567,27 +1574,23 @@ protected void doRun() {
                         state = getHandler().process(socketWrapper, event);
                     }
                     if (state == SocketState.CLOSED) {
-                        SelectionKey key = JreCompat.isJre11Available() ? null : socket.getIOChannel().keyFor(poller.getSelector());
-                        poller.cancelledKey(key, socketWrapper);
+                        poller.cancelledKey(getSelectionKey(), socketWrapper);
                     }
                 } else if (handshake == -1 ) {
                     getHandler().process(socketWrapper, SocketEvent.CONNECT_FAIL);
-                    SelectionKey key = JreCompat.isJre11Available() ? null : socket.getIOChannel().keyFor(poller.getSelector());
-                    poller.cancelledKey(key, socketWrapper);
+                    poller.cancelledKey(getSelectionKey(), socketWrapper);
                 } else if (handshake == SelectionKey.OP_READ){
                     socketWrapper.registerReadInterest();
                 } else if (handshake == SelectionKey.OP_WRITE){
                     socketWrapper.registerWriteInterest();
                 }
             } catch (CancelledKeyException cx) {
-                SelectionKey key = JreCompat.isJre11Available() ? null : socket.getIOChannel().keyFor(poller.getSelector());
-                poller.cancelledKey(key, socketWrapper);
+                poller.cancelledKey(getSelectionKey(), socketWrapper);
             } catch (VirtualMachineError vme) {
                 ExceptionUtils.handleThrowable(vme);
             } catch (Throwable t) {
                 log.error(sm.getString("endpoint.processing.fail"), t);
-                SelectionKey key = JreCompat.isJre11Available() ? null : socket.getIOChannel().keyFor(poller.getSelector());
-                poller.cancelledKey(key, socketWrapper);
+                poller.cancelledKey(getSelectionKey(), socketWrapper);
             } finally {
                 socketWrapper = null;
                 event = null;
@@ -1597,8 +1600,23 @@ protected void doRun() {
                 }
             }
         }
+
+        private SelectionKey getSelectionKey() {
+            // Shortcut for Java 11 onwards
+            if (JreCompat.isJre11Available()) {
+                return null;
+            }
+
+            SocketChannel socketChannel = socketWrapper.getSocket().getIOChannel();
+            if (socketChannel == null) {
+                return null;
+            }
+
+            return socketChannel.keyFor(NioEndpoint.this.poller.getSelector());
+        }
     }
 
+
     // ----------------------------------------------- SendfileData Inner Class
 
     /**

test/org/apache/tomcat/websocket/TestWebSocketFrameClient.java

@@ -23,8 +23,6 @@
 import java.util.Queue;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
-import java.util.logging.Level;
-import java.util.logging.LogManager;
 
 import jakarta.websocket.ClientEndpointConfig;
 import jakarta.websocket.ClientEndpointConfig.Configurator;
@@ -117,20 +115,11 @@ public void testConnectToRootEndpoint() throws Exception {
 
         tomcat.start();
 
-        LogManager.getLogManager().getLogger("org.apache.coyote").setLevel(Level.ALL);
-        LogManager.getLogManager().getLogger("org.apache.tomcat.websocket").setLevel(Level.ALL);
-        LogManager.getLogManager().getLogger("org.apache.tomcat.util.net").setLevel(Level.ALL);
-        try {
-            echoTester("",null);
-            echoTester("/",null);
-            // This will trigger a redirect so there will be 5 requests logged
-            echoTester("/foo",null);
-            echoTester("/foo/",null);
-        } finally {
-            LogManager.getLogManager().getLogger("org.apache.coyote").setLevel(Level.INFO);
-            LogManager.getLogManager().getLogger("org.apache.tomcat.websocket.WsWebSocketContainer").setLevel(Level.INFO);
-            LogManager.getLogManager().getLogger("org.apache.tomcat.util.net").setLevel(Level.INFO);
-        }
+        echoTester("",null);
+        echoTester("/",null);
+        // This will trigger a redirect so there will be 5 requests logged
+        echoTester("/foo",null);
+        echoTester("/foo/",null);
     }
 
     public void echoTester(String path, ClientEndpointConfig clientEndpointConfig)
@@ -198,7 +187,6 @@ public void testConnectToBasicEndpoint() throws Exception {
         clientEndpointConfig.getUserProperties().put(Constants.WS_AUTHENTICATION_PASSWORD, utf8Pass);
 
         echoTester(URI_PROTECTED, clientEndpointConfig);
-
     }
 
     @Test
@@ -235,7 +223,5 @@ public void testConnectToDigestEndpoint() throws Exception {
         clientEndpointConfig.getUserProperties().put(Constants.WS_AUTHENTICATION_PASSWORD,PWD);
 
         echoTester(URI_PROTECTED, clientEndpointConfig);
-
     }
-
 }

webapps/docs/changelog.xml

@@ -144,6 +144,10 @@
       <fix>
         <bug>64830</bug>: Fix concurrency issue in HPACK decoder. (markt)
       </fix>
+      <fix>
+        Fix a concurrency issue in the NIO connector that could cause newly
+        created connections to be removed from the poller. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Jasper">