From 87942cb23614f94bbc6db0df3dd498926c676de9 Mon Sep 17 00:00:00 2001 From: markt Date: Wed, 23 Feb 2011 11:58:47 +0000 Subject: [PATCH] Better handling in acceptor threads if server hits ulimit for open files git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1073711 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/tomcat/util/net/AbstractEndpoint.java | 35 +++++++++++++++++++++- java/org/apache/tomcat/util/net/AprEndpoint.java | 19 ++++++++++-- java/org/apache/tomcat/util/net/JIoEndpoint.java | 22 +++++++++++--- java/org/apache/tomcat/util/net/NioEndpoint.java | 21 +++++++++++-- webapps/docs/changelog.xml | 8 +++++ 5 files changed, 95 insertions(+), 10 deletions(-) diff --git a/java/org/apache/tomcat/util/net/AbstractEndpoint.java b/java/org/apache/tomcat/util/net/AbstractEndpoint.java index 98b76badd..b9e034b35 100644 --- a/java/org/apache/tomcat/util/net/AbstractEndpoint.java +++ b/java/org/apache/tomcat/util/net/AbstractEndpoint.java @@ -104,6 +104,9 @@ public abstract class AbstractEndpoint { public static final String SSL_ATTR_ALLOW_UNSAFE_RENEG = "allowUnsafeLegacyRenegotiation"; + private static final int INITIAL_ERROR_DELAY = 50; + private static final int MAX_ERROR_DELAY = 1600; + // ----------------------------------------------------------------- Fields @@ -614,7 +617,37 @@ public abstract class AbstractEndpoint { } else return -1; } - + /** + * Provides a common approach for sub-classes to handle exceptions where a + * delay is required to prevent a Thread from entering a tight loop which + * will consume CPU and may also trigger large amounts of logging. For + * example, this can happen with the Acceptor thread if the ulimit for open + * files is reached. + * + * @param currentErrorDelay The current delay beign applied on failure + * @return The delay to apply on the next failure + */ + protected int handleExceptionWithDelay(int currentErrorDelay) { + // Don't delay on first exception + if (currentErrorDelay > 0) { + try { + Thread.sleep(currentErrorDelay); + } catch (InterruptedException e) { + // Ignore + } + } + + // On subsequent exceptions, start the delay at 50ms, doubling the delay + // on every subsequent exception until the delay reaches 1.6 seconds. + if (currentErrorDelay == 0) { + return INITIAL_ERROR_DELAY; + } else if (currentErrorDelay < MAX_ERROR_DELAY) { + return currentErrorDelay * 2; + } else { + return MAX_ERROR_DELAY; + } + + } // -------------------- SSL related properties -------------------- diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java b/java/org/apache/tomcat/util/net/AprEndpoint.java index 1d0ef85bc..e0649b4e4 100644 --- a/java/org/apache/tomcat/util/net/AprEndpoint.java +++ b/java/org/apache/tomcat/util/net/AprEndpoint.java @@ -914,6 +914,8 @@ public class AprEndpoint extends AbstractEndpoint { @Override public void run() { + int errorDelay = 0; + // Loop until we receive a shutdown command while (running) { @@ -932,8 +934,21 @@ public class AprEndpoint extends AbstractEndpoint { try { //if we have reached max connections, wait awaitConnection(); - // Accept the next incoming connection from the server socket - long socket = Socket.accept(serverSock); + + long socket = 0; + try { + // Accept the next incoming connection from the server + // socket + socket = Socket.accept(serverSock); + } catch (Exception e) { + // Introduce delay if necessary + errorDelay = handleExceptionWithDelay(errorDelay); + // re-throw + throw e; + } + // Successful accept, reset the error delay + errorDelay = 0; + //increment socket count countUpConnection(); /* diff --git a/java/org/apache/tomcat/util/net/JIoEndpoint.java b/java/org/apache/tomcat/util/net/JIoEndpoint.java index 81d01160f..3e97b4e44 100644 --- a/java/org/apache/tomcat/util/net/JIoEndpoint.java +++ b/java/org/apache/tomcat/util/net/JIoEndpoint.java @@ -183,6 +183,8 @@ public class JIoEndpoint extends AbstractEndpoint { @Override public void run() { + int errorDelay = 0; + // Loop until we receive a shutdown command while (running) { @@ -200,10 +202,22 @@ public class JIoEndpoint extends AbstractEndpoint { } try { //if we have reached max connections, wait - awaitConnection(); - // Accept the next incoming connection from the server socket - Socket socket = serverSocketFactory.acceptSocket(serverSocket); - + awaitConnection(); + + Socket socket = null; + try { + // Accept the next incoming connection from the server + // socket + socket = serverSocketFactory.acceptSocket(serverSocket); + } catch (IOException ioe) { + // Introduce delay if necessary + errorDelay = handleExceptionWithDelay(errorDelay); + // re-throw + throw ioe; + } + // Successful accept, reset the error delay + errorDelay = 0; + // Configure the socket if (setSocketOptions(socket)) { // Hand this socket off to an appropriate processor diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java index aba8ca20e..59ee0ed1f 100644 --- a/java/org/apache/tomcat/util/net/NioEndpoint.java +++ b/java/org/apache/tomcat/util/net/NioEndpoint.java @@ -794,7 +794,9 @@ public class NioEndpoint extends AbstractEndpoint { */ @Override public void run() { - + + int errorDelay = 0; + // Loop until we receive a shutdown command while (running) { @@ -813,8 +815,21 @@ public class NioEndpoint extends AbstractEndpoint { try { //if we have reached max connections, wait awaitConnection(); - // Accept the next incoming connection from the server socket - SocketChannel socket = serverSock.accept(); + + SocketChannel socket = null; + try { + // Accept the next incoming connection from the server + // socket + socket = serverSock.accept(); + } catch (IOException ioe) { + // Introduce delay if necessary + errorDelay = handleExceptionWithDelay(errorDelay); + // re-throw + throw ioe; + } + // Successful accept, reset the error delay + errorDelay = 0; + // Hand this socket off to an appropriate processor //TODO FIXME - this is currently a blocking call, meaning we will be blocking //further accepts until there is a thread available. diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index bdb9ceb71..21eaab9b9 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -130,6 +130,14 @@ 50780: Fix memory leak in APR implementation of AJP connector introduced by the refactoring for 49884. (markt) + + If server configuration errors and/or faulty applications caused the + ulimit for open files to be reached, the acceptor threads for all + connectors could enter a tight loop. This loop consumed CPU and also + logged an error message for every iteration of the loop which lead to + large log files being generated. The acceptors have been enhanced to + better handle this situation. (markt) + -- 2.11.0