This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 8.5.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push: new 846ff9a Improve clean-up after an OOME during request processing 846ff9a is described below commit 846ff9afb4bd3c1fec582886fb183c83e1ca0b70 Author: Mark Thomas <ma...@apache.org> AuthorDate: Mon May 13 20:10:13 2019 +0100 Improve clean-up after an OOME during request processing --- .../apache/catalina/core/StandardWrapperValve.java | 77 +++++++++++----------- java/org/apache/coyote/AbstractProtocol.java | 8 ++- java/org/apache/coyote/LocalStrings.properties | 1 + webapps/docs/changelog.xml | 4 ++ 4 files changed, 50 insertions(+), 40 deletions(-) diff --git a/java/org/apache/catalina/core/StandardWrapperValve.java b/java/org/apache/catalina/core/StandardWrapperValve.java index ffcba85..c7b7590 100644 --- a/java/org/apache/catalina/core/StandardWrapperValve.java +++ b/java/org/apache/catalina/core/StandardWrapperValve.java @@ -253,50 +253,49 @@ final class StandardWrapperValve context.getName()), e); throwable = e; exception(request, response, e); - } - - // Release the filter chain (if any) for this request - if (filterChain != null) { - filterChain.release(); - } - - // Deallocate the allocated servlet instance - try { - if (servlet != null) { - wrapper.deallocate(servlet); + } finally { + // Release the filter chain (if any) for this request + if (filterChain != null) { + filterChain.release(); } - } catch (Throwable e) { - ExceptionUtils.handleThrowable(e); - container.getLogger().error(sm.getString("standardWrapper.deallocateException", - wrapper.getName()), e); - if (throwable == null) { - throwable = e; - exception(request, response, e); - } - } - // If this servlet has been marked permanently unavailable, - // unload it and release this instance - try { - if ((servlet != null) && - (wrapper.getAvailable() == Long.MAX_VALUE)) { - wrapper.unload(); - } - } catch (Throwable e) { - ExceptionUtils.handleThrowable(e); - container.getLogger().error(sm.getString("standardWrapper.unloadException", - wrapper.getName()), e); - if (throwable == null) { - exception(request, response, e); + // Deallocate the allocated servlet instance + try { + if (servlet != null) { + wrapper.deallocate(servlet); + } + } catch (Throwable e) { + ExceptionUtils.handleThrowable(e); + container.getLogger().error(sm.getString("standardWrapper.deallocateException", + wrapper.getName()), e); + if (throwable == null) { + throwable = e; + exception(request, response, e); + } } - } - long t2=System.currentTimeMillis(); - long time=t2-t1; - processingTime += time; - if( time > maxTime) maxTime=time; - if( time < minTime) minTime=time; + // If this servlet has been marked permanently unavailable, + // unload it and release this instance + try { + if ((servlet != null) && + (wrapper.getAvailable() == Long.MAX_VALUE)) { + wrapper.unload(); + } + } catch (Throwable e) { + ExceptionUtils.handleThrowable(e); + container.getLogger().error(sm.getString("standardWrapper.unloadException", + wrapper.getName()), e); + if (throwable == null) { + exception(request, response, e); + } + } + long t2=System.currentTimeMillis(); + long time=t2-t1; + processingTime += time; + if( time > maxTime) maxTime=time; + if( time < minTime) minTime=time; + } } diff --git a/java/org/apache/coyote/AbstractProtocol.java b/java/org/apache/coyote/AbstractProtocol.java index be2bd40..684582f 100644 --- a/java/org/apache/coyote/AbstractProtocol.java +++ b/java/org/apache/coyote/AbstractProtocol.java @@ -945,7 +945,13 @@ public abstract class AbstractProtocol<S> implements ProtocolHandler, // Future developers: if you discover any other // rare-but-nonfatal exceptions, catch them here, and log as // above. - catch (Throwable e) { + catch (OutOfMemoryError oome) { + // Try and handle this here to give Tomcat a chance to close the + // connection and prevent clients waiting until they time out. + // Worst case, it isn't recoverable and the attempt at logging + // will trigger another OOME. + getLog().error(sm.getString("abstractConnectionHandler.oome"), oome); + } catch (Throwable e) { ExceptionUtils.handleThrowable(e); // any other exception or error is odd. Here we log it // with "ERROR" level, so it will show up even on diff --git a/java/org/apache/coyote/LocalStrings.properties b/java/org/apache/coyote/LocalStrings.properties index 4c59741..33fc21f 100644 --- a/java/org/apache/coyote/LocalStrings.properties +++ b/java/org/apache/coyote/LocalStrings.properties @@ -17,6 +17,7 @@ abstractConnectionHandler.connectionsGet=Found processor [{0}] for socket [{1}] abstractConnectionHandler.error=Error reading request, ignored abstractConnectionHandler.ioexception.debug=IOExceptions are normal, ignored abstractConnectionHandler.negotiatedProcessor.fail=Failed to create Processor for negotiated protocol [{0}] +abstractConnectionHandler.oome=Failed to complete processing of a request abstractConnectionHandler.process=Processing socket [{0}] with status [{1}] abstractConnectionHandler.processorPop=Popped processor [{0}] from cache abstractConnectionHandler.protocolexception.debug=ProtocolExceptions are normal, ignored diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 0b50e22..41c67ec 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -60,6 +60,10 @@ set using a canonical path which in turn meant resource URLs were not being constructed as expected. (markt) </fix> + <fix> + Make a best efforts attempt to clean-up if a request fails during + processing due to an <code>OutOfMemoryException</code>. (markt) + </fix> </changelog> </subsection> <subsection name="Coyote"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org