From 73265a03cc7140aecf35387a24d4a128b9938677 Mon Sep 17 00:00:00 2001 From: markt Date: Mon, 28 Mar 2011 19:15:06 +0000 Subject: [PATCH] Move the processor.recycle calls to just before the point where the processor is returned to the pool. This ensures returned processors are recycled (this could have been skipped on some exception paths) Possible contributing factor to https://issues.apache.org/bugzilla/show_bug.cgi?id=50957 ? git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1086349 13f79535-47bb-0310-9956-ffa450edef68 --- java/org/apache/coyote/ajp/AjpAprProcessor.java | 5 ----- java/org/apache/coyote/ajp/AjpAprProtocol.java | 3 +++ java/org/apache/coyote/ajp/AjpProcessor.java | 4 ---- java/org/apache/coyote/ajp/AjpProtocol.java | 2 ++ java/org/apache/coyote/http11/Http11AprProcessor.java | 6 ------ java/org/apache/coyote/http11/Http11AprProtocol.java | 4 ++++ java/org/apache/coyote/http11/Http11NioProcessor.java | 17 ++++------------- java/org/apache/coyote/http11/Http11Processor.java | 2 -- java/org/apache/coyote/http11/Http11Protocol.java | 2 ++ 9 files changed, 15 insertions(+), 30 deletions(-) diff --git a/java/org/apache/coyote/ajp/AjpAprProcessor.java b/java/org/apache/coyote/ajp/AjpAprProcessor.java index 764762079..f3b9e8c7b 100644 --- a/java/org/apache/coyote/ajp/AjpAprProcessor.java +++ b/java/org/apache/coyote/ajp/AjpAprProcessor.java @@ -316,7 +316,6 @@ public class AjpAprProcessor extends AbstractAjpProcessor { rp.setStage(org.apache.coyote.Constants.STAGE_KEEPALIVE); recycle(); - } // Add the socket to the poller @@ -329,12 +328,10 @@ public class AjpAprProcessor extends AbstractAjpProcessor { rp.setStage(org.apache.coyote.Constants.STAGE_ENDED); if (error || endpoint.isPaused()) { - recycle(); return SocketState.CLOSED; } else if (isAsync()) { return SocketState.LONG; } else { - recycle(); return SocketState.OPEN; } } @@ -369,14 +366,12 @@ public class AjpAprProcessor extends AbstractAjpProcessor { if (isAsync()) { if (error) { request.updateCounters(); - recycle(); return SocketState.CLOSED; } else { return SocketState.LONG; } } else { request.updateCounters(); - recycle(); if (error) { return SocketState.CLOSED; } else { diff --git a/java/org/apache/coyote/ajp/AjpAprProtocol.java b/java/org/apache/coyote/ajp/AjpAprProtocol.java index 13182a2c5..68facefbc 100644 --- a/java/org/apache/coyote/ajp/AjpAprProtocol.java +++ b/java/org/apache/coyote/ajp/AjpAprProtocol.java @@ -197,6 +197,7 @@ public class AjpAprProtocol extends AbstractAjpProtocol { connections.put(socket, processor); socket.setAsync(true); } else { + processor.recycle(); recycledProcessors.offer(processor); } return state; @@ -220,6 +221,7 @@ public class AjpAprProtocol extends AbstractAjpProtocol { // less-than-verbose logs. log.error(sm.getString("ajpprotocol.proto.error"), e); } + processor.recycle(); recycledProcessors.offer(processor); return SocketState.CLOSED; } @@ -251,6 +253,7 @@ public class AjpAprProtocol extends AbstractAjpProtocol { } if (state != SocketState.LONG && state != SocketState.ASYNC_END) { connections.remove(socket); + processor.recycle(); recycledProcessors.offer(processor); if (state == SocketState.OPEN) { ((AprEndpoint)proto.endpoint).getPoller().add(socket.getSocket().longValue()); diff --git a/java/org/apache/coyote/ajp/AjpProcessor.java b/java/org/apache/coyote/ajp/AjpProcessor.java index b018b028a..c4023d2c9 100644 --- a/java/org/apache/coyote/ajp/AjpProcessor.java +++ b/java/org/apache/coyote/ajp/AjpProcessor.java @@ -334,7 +334,6 @@ public class AjpProcessor extends AbstractAjpProcessor { rp.setStage(org.apache.coyote.Constants.STAGE_KEEPALIVE); recycle(); - } rp.setStage(org.apache.coyote.Constants.STAGE_ENDED); @@ -342,7 +341,6 @@ public class AjpProcessor extends AbstractAjpProcessor { if (isAsync() && !error && !endpoint.isPaused()) { return SocketState.LONG; } else { - recycle(); input = null; output = null; return SocketState.CLOSED; @@ -373,7 +371,6 @@ public class AjpProcessor extends AbstractAjpProcessor { if (error) { response.setStatus(500); request.updateCounters(); - recycle(); input = null; output = null; return SocketState.CLOSED; @@ -385,7 +382,6 @@ public class AjpProcessor extends AbstractAjpProcessor { response.setStatus(500); } request.updateCounters(); - recycle(); input = null; output = null; return SocketState.CLOSED; diff --git a/java/org/apache/coyote/ajp/AjpProtocol.java b/java/org/apache/coyote/ajp/AjpProtocol.java index 95dcafd03..26644e743 100644 --- a/java/org/apache/coyote/ajp/AjpProtocol.java +++ b/java/org/apache/coyote/ajp/AjpProtocol.java @@ -189,6 +189,7 @@ public class AjpProtocol extends AbstractAjpProtocol { return processor.asyncPostProcess(); } else { socket.setAsync(false); + processor.recycle(); recycledProcessors.offer(processor); } return state; @@ -211,6 +212,7 @@ public class AjpProtocol extends AbstractAjpProtocol { // less-than-verbose logs. log.error(sm.getString("ajpprotocol.proto.error"), e); } + processor.recycle(); recycledProcessors.offer(processor); return SocketState.CLOSED; } diff --git a/java/org/apache/coyote/http11/Http11AprProcessor.java b/java/org/apache/coyote/http11/Http11AprProcessor.java index 2f4f1aabd..98d15d0e5 100644 --- a/java/org/apache/coyote/http11/Http11AprProcessor.java +++ b/java/org/apache/coyote/http11/Http11AprProcessor.java @@ -187,12 +187,10 @@ public class Http11AprProcessor extends AbstractHttp11Processor { if (error) { inputBuffer.nextRequest(); outputBuffer.nextRequest(); - recycle(); return SocketState.CLOSED; } else if (!comet) { inputBuffer.nextRequest(); outputBuffer.nextRequest(); - recycle(); return SocketState.OPEN; } else { return SocketState.LONG; @@ -367,12 +365,10 @@ public class Http11AprProcessor extends AbstractHttp11Processor { rp.setStage(org.apache.coyote.Constants.STAGE_ENDED); if (error || endpoint.isPaused()) { - recycle(); return SocketState.CLOSED; } else if (comet || isAsync()) { return SocketState.LONG; } else { - recycle(); return (openSocket) ? SocketState.OPEN : SocketState.CLOSED; } @@ -406,12 +402,10 @@ public class Http11AprProcessor extends AbstractHttp11Processor { rp.setStage(org.apache.coyote.Constants.STAGE_ENDED); if (error) { - recycle(); return SocketState.CLOSED; } else if (isAsync()) { return SocketState.LONG; } else { - recycle(); if (!keepAlive) { return SocketState.CLOSED; } else { diff --git a/java/org/apache/coyote/http11/Http11AprProtocol.java b/java/org/apache/coyote/http11/Http11AprProtocol.java index d4db68201..a112b9ddd 100644 --- a/java/org/apache/coyote/http11/Http11AprProtocol.java +++ b/java/org/apache/coyote/http11/Http11AprProtocol.java @@ -296,6 +296,7 @@ public class Http11AprProtocol extends AbstractHttp11Protocol { if (state != SocketState.LONG) { connections.remove(socket.getSocket()); socket.setAsync(false); + processor.recycle(); recycledProcessors.offer(processor); if (state == SocketState.OPEN) { ((AprEndpoint)proto.endpoint).getPoller().add(socket.getSocket().longValue()); @@ -337,6 +338,7 @@ public class Http11AprProtocol extends AbstractHttp11Protocol { socket.getSocket().longValue()); } } else { + processor.recycle(); recycledProcessors.offer(processor); } return state; @@ -361,6 +363,7 @@ public class Http11AprProtocol extends AbstractHttp11Protocol { Http11AprProtocol.log.error( sm.getString("http11protocol.proto.error"), e); } + processor.recycle(); recycledProcessors.offer(processor); return SocketState.CLOSED; } @@ -391,6 +394,7 @@ public class Http11AprProtocol extends AbstractHttp11Protocol { if (state != SocketState.LONG && state != SocketState.ASYNC_END) { connections.remove(socket.getSocket()); socket.setAsync(false); + processor.recycle(); recycledProcessors.offer(processor); if (state == SocketState.OPEN) { ((AprEndpoint)proto.endpoint).getPoller().add(socket.getSocket().longValue()); diff --git a/java/org/apache/coyote/http11/Http11NioProcessor.java b/java/org/apache/coyote/http11/Http11NioProcessor.java index edbbae3d1..e774b7f1d 100644 --- a/java/org/apache/coyote/http11/Http11NioProcessor.java +++ b/java/org/apache/coyote/http11/Http11NioProcessor.java @@ -206,10 +206,8 @@ public class Http11NioProcessor extends AbstractHttp11Processor { rp.setStage(org.apache.coyote.Constants.STAGE_ENDED); if (error) { - recycle(); return SocketState.CLOSED; } else if (!comet) { - recycle(); return (keepAlive)?SocketState.OPEN:SocketState.CLOSED; } else { return SocketState.LONG; @@ -267,10 +265,8 @@ public class Http11NioProcessor extends AbstractHttp11Processor { rp.setStage(org.apache.coyote.Constants.STAGE_ENDED); if (error) { - recycle(); return SocketState.CLOSED; } else if (!comet && !isAsync()) { - recycle(); return (keepAlive)?SocketState.OPEN:SocketState.CLOSED; } else { return SocketState.LONG; @@ -305,7 +301,7 @@ public class Http11NioProcessor extends AbstractHttp11Processor { boolean keptAlive = false; boolean openSocket = false; - boolean recycle = true; + boolean readComplete = true; final KeyAttachment ka = (KeyAttachment)socket.getAttachment(false); while (!error && keepAlive && !comet && !isAsync() && !endpoint.isPaused()) { @@ -328,7 +324,7 @@ public class Http11NioProcessor extends AbstractHttp11Processor { } else { // Started to read request line. Need to keep processor // associated with socket - recycle = false; + readComplete = false; } if (endpoint.isPaused()) { // 503 - Service unavailable @@ -345,7 +341,7 @@ public class Http11NioProcessor extends AbstractHttp11Processor { //we've read part of the request, don't recycle it //instead associate it with the socket openSocket = true; - recycle = false; + readComplete = false; break; } request.setStartTime(System.currentTimeMillis()); @@ -471,16 +467,11 @@ public class Http11NioProcessor extends AbstractHttp11Processor { rp.setStage(org.apache.coyote.Constants.STAGE_ENDED); if (error || endpoint.isPaused()) { - recycle(); return SocketState.CLOSED; } else if (comet || isAsync()) { return SocketState.LONG; } else { - if (recycle) { - recycle(); - } - //return (openSocket) ? (SocketState.OPEN) : SocketState.CLOSED; - return (openSocket) ? (recycle?SocketState.OPEN:SocketState.LONG) : SocketState.CLOSED; + return (openSocket) ? (readComplete?SocketState.OPEN:SocketState.LONG) : SocketState.CLOSED; } } diff --git a/java/org/apache/coyote/http11/Http11Processor.java b/java/org/apache/coyote/http11/Http11Processor.java index 9afad5456..cc95597d6 100644 --- a/java/org/apache/coyote/http11/Http11Processor.java +++ b/java/org/apache/coyote/http11/Http11Processor.java @@ -356,12 +356,10 @@ public class Http11Processor extends AbstractHttp11Processor { rp.setStage(org.apache.coyote.Constants.STAGE_ENDED); if (error) { - recycle(); return SocketState.CLOSED; } else if (isAsync()) { return SocketState.LONG; } else { - recycle(); if (!keepAlive) { return SocketState.CLOSED; } else { diff --git a/java/org/apache/coyote/http11/Http11Protocol.java b/java/org/apache/coyote/http11/Http11Protocol.java index e8c6e94f2..23da8065b 100644 --- a/java/org/apache/coyote/http11/Http11Protocol.java +++ b/java/org/apache/coyote/http11/Http11Protocol.java @@ -194,6 +194,7 @@ public class Http11Protocol extends AbstractHttp11JsseProtocol { return processor.asyncPostProcess(); } else { socket.setAsync(false); + processor.recycle(); recycledProcessors.offer(processor); } return state; @@ -216,6 +217,7 @@ public class Http11Protocol extends AbstractHttp11JsseProtocol { // less-than-verbose logs. log.error(sm.getString("http11protocol.proto.error"), e); } + processor.recycle(); recycledProcessors.offer(processor); return SocketState.CLOSED; } -- 2.11.0