fix issue Dispatcher thread stuck in SSL Handshake, httpserver not anymore responsive robaho#23

robaho · robaho · commit e7d678c86e59 · 2025-06-04T12:45:56.000-05:00
diff --git a/src/main/java/robaho/net/httpserver/ServerImpl.java b/src/main/java/robaho/net/httpserver/ServerImpl.java
@@ -342,82 +342,84 @@ public void run() {
             while (true) {
                 try {
                     Socket s = socket.accept();
-                    if(logger.isLoggable(Level.TRACE)) {
-                        logger.log(Level.TRACE, "accepted connection: " + s.toString());
-                    }
-                    stats.connectionCount.incrementAndGet();
-                    if (MAX_CONNECTIONS > 0 && allConnections.size() >= MAX_CONNECTIONS) {
-                        // we've hit max limit of current open connections, so we go
-                        // ahead and close this connection without processing it
+                    executor.execute(() -> {
                         try {
-                            stats.maxConnectionsExceededCount.incrementAndGet();
-                            logger.log(Level.WARNING, "closing accepted connection due to too many connections");
-                            s.close();
-                        } catch (IOException ignore) {
+                            acceptConnection(s);
+                        } catch (IOException t) {
+                            logger.log(Level.ERROR, "Dispatcher Exception", t);
                         }
-                        continue;
-                    }
-
-                    if (ServerConfig.noDelay()) {
-                        s.setTcpNoDelay(true);
+                    });
+                } catch (IOException e) {
+                    if (!isFinishing()) {
+                        logger.log(Level.ERROR, "Dispatcher Exception, terminating", e);
                     }
+                    return;
+                }
+            }
+        }
+        private void acceptConnection(Socket s) throws IOException {
+            if(logger.isLoggable(Level.TRACE)) {
+                logger.log(Level.TRACE, "accepted connection: " + s.toString());
+            }
+            stats.connectionCount.incrementAndGet();
+            if (MAX_CONNECTIONS > 0 && allConnections.size() >= MAX_CONNECTIONS) {
+                // we've hit max limit of current open connections, so we go
+                // ahead and close this connection without processing it
+                try {
+                    stats.maxConnectionsExceededCount.incrementAndGet();
+                    logger.log(Level.WARNING, "closing accepted connection due to too many connections");
+                    s.close();
+                } catch (IOException ignore) {
+                }
+                return;
+            }
 
-                    boolean http2 = false;
+            if (ServerConfig.noDelay()) {
+                s.setTcpNoDelay(true);
+            }
 
-                    if (https) {
-                        // for some reason, creating an SSLServerSocket and setting the default parameters would
-                        // not work, so upgrade to a SSLSocket after connection
-                        SSLSocketFactory ssf = httpsConfig.getSSLContext().getSocketFactory();
-                        SSLSocket sslSocket = (SSLSocket) ssf.createSocket(s, null, false);
-                        SSLConfigurator.configure(sslSocket,httpsConfig);
+            boolean http2 = false;
 
-                        sslSocket.setHandshakeApplicationProtocolSelector((_sslSocket, protocols) -> {
-                            if (protocols.contains("h2") && ServerConfig.http2OverSSL()) {
-                                return "h2";
-                            } else {
-                                return "http/1.1";
-                            }
-                        });
-                        // the following forces the SSL handshake to complete in order to determine the negotiated protocol
-                        var session = sslSocket.getSession();
-                        if ("h2".equals(sslSocket.getApplicationProtocol())) {
-                            logger.log(Level.DEBUG, () -> "http2 connection "+sslSocket.toString());
-                            http2 = true;
-                        } else {
-                            logger.log(Level.DEBUG, () -> "http/1.1 connection "+sslSocket.toString());
-                        }
-                        s = sslSocket;
+            if (https) {
+                // for some reason, creating an SSLServerSocket and setting the default parameters would
+                // not work, so upgrade to a SSLSocket after connection
+                SSLSocketFactory ssf = httpsConfig.getSSLContext().getSocketFactory();
+                SSLSocket sslSocket = (SSLSocket) ssf.createSocket(s, null, false);
+                SSLConfigurator.configure(sslSocket,httpsConfig);
+
+                sslSocket.setHandshakeApplicationProtocolSelector((_sslSocket, protocols) -> {
+                    if (protocols.contains("h2") && ServerConfig.http2OverSSL()) {
+                        return "h2";
+                    } else {
+                        return "http/1.1";
                     }
+                });
+                // the following forces the SSL handshake to complete in order to determine the negotiated protocol
+                var session = sslSocket.getSession();
+                if ("h2".equals(sslSocket.getApplicationProtocol())) {
+                    logger.log(Level.DEBUG, () -> "http2 connection "+sslSocket.toString());
+                    http2 = true;
+                } else {
+                    logger.log(Level.DEBUG, () -> "http/1.1 connection "+sslSocket.toString());
+                }
+                s = sslSocket;
+            }
 
-                    HttpConnection c;
-                    try {
-                        c = new HttpConnection(s);
-                    } catch (IOException e) {
-                        logger.log(Level.WARNING, "Failed to create HttpConnection", e);
-                        continue;
-                    }
-                    try {
-                        allConnections.add(c);
-
-                        if (http2) {
-                            Http2Exchange t = new Http2Exchange(protocol, c);
-                            executor.execute(t);
-                        } else {
-                            Exchange t = new Exchange(protocol, c);
-                            executor.execute(t);
-                        }
+            HttpConnection c = new HttpConnection(s);
+            try {
+                allConnections.add(c);
 
-                    } catch (Exception e) {
-                        logger.log(Level.TRACE, "Dispatcher Exception", e);
-                        stats.handleExceptionCount.incrementAndGet();
-                        closeConnection(c);
-                    }
-                } catch (IOException e) {
-                    if (!isFinishing()) {
-                        logger.log(Level.ERROR, "Dispatcher Exception, terminating", e);
-                    }
-                    return;
+                if (http2) {
+                    Http2Exchange t = new Http2Exchange(protocol, c);
+                    executor.execute(t);
+                } else {
+                    Exchange t = new Exchange(protocol, c);
+                    executor.execute(t);
                 }
+            } catch (Exception e) {
+                logger.log(Level.TRACE, "Dispatcher Exception", e);
+                stats.handleExceptionCount.incrementAndGet();
+                closeConnection(c);
             }
         }
     }
