You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2014/06/13 22:21:33 UTC
svn commit: r1602510 - in /tomcat/tc7.0.x/trunk: ./
java/org/apache/catalina/connector/ java/org/apache/coyote/
java/org/apache/coyote/ajp/ java/org/apache/coyote/http11/
java/org/apache/coyote/http11/upgrade/ java/org/apache/tomcat/util/net/
java/org/...
Author: markt
Date: Fri Jun 13 20:21:32 2014
New Revision: 1602510
URL: http://svn.apache.org/r1602510
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56518
Do not attempt an NIO write if a thread has been interrupted as it can lead to a connection limit leak
Modified:
tomcat/tc7.0.x/trunk/ (props changed)
tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
tomcat/tc7.0.x/trunk/java/org/apache/coyote/AbstractProcessor.java
tomcat/tc7.0.x/trunk/java/org/apache/coyote/Adapter.java
tomcat/tc7.0.x/trunk/java/org/apache/coyote/LocalStrings.properties
tomcat/tc7.0.x/trunk/java/org/apache/coyote/Processor.java
tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java
tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java
tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpNioProcessor.java
tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/upgrade/AbstractProcessor.java
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/upgrade/UpgradeProcessor.java
tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java
tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/NioChannel.java
tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java
tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java
tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/SocketStatus.java
tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/res/LocalStrings.properties
tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
Merged /tomcat/trunk:r1602198
Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java?rev=1602510&r1=1602509&r2=1602510&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java Fri Jun 13 20:21:32 2014
@@ -469,6 +469,34 @@ public class CoyoteAdapter implements Ad
@Override
+ public void errorDispatch(org.apache.coyote.Request req,
+ org.apache.coyote.Response res) {
+ Request request = (Request) req.getNote(ADAPTER_NOTES);
+ Response response = (Response) res.getNote(ADAPTER_NOTES);
+
+ if (request != null && request.getMappingData().context != null) {
+ ((Context) request.getMappingData().context).logAccess(
+ request, response,
+ System.currentTimeMillis() - req.getStartTime(),
+ false);
+ } else {
+ log(req, res, System.currentTimeMillis() - req.getStartTime());
+ }
+
+ if (request != null) {
+ request.recycle();
+ }
+
+ if (response != null) {
+ response.recycle();
+ }
+
+ res.recycle();
+ res.recycle();
+ }
+
+
+ @Override
public void log(org.apache.coyote.Request req,
org.apache.coyote.Response res, long time) {
Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/AbstractProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/AbstractProcessor.java?rev=1602510&r1=1602509&r2=1602510&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/coyote/AbstractProcessor.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/AbstractProcessor.java Fri Jun 13 20:21:32 2014
@@ -69,8 +69,20 @@ public abstract class AbstractProcessor<
* Update the current error state to the new error state if the new error
* state is more severe than the current error state.
*/
- protected void setErrorState(ErrorState errorState) {
+ protected void setErrorState(ErrorState errorState, Throwable t) {
+ boolean blockIo = this.errorState.isIoAllowed() && !errorState.isIoAllowed();
this.errorState = this.errorState.getMostSevere(errorState);
+ if (blockIo && !ContainerThreadMarker.isContainerThread()) {
+ // The error occurred on a non-container thread which means not all
+ // of the necessary clean-up will have been completed. Dispatch to
+ // a container thread to do the clean-up. Need to do it this way to
+ // ensure that all the necessary clean-up is performed.
+ if (response.getStatus() < 400) {
+ response.setStatus(500);
+ }
+ getLog().info(sm.getString("abstractProcessor.nonContainerThreadError"), t);
+ getEndpoint().processSocketAsync(socketWrapper, SocketStatus.CLOSE_NOW);
+ }
}
@@ -157,6 +169,11 @@ public abstract class AbstractProcessor<
}
@Override
+ public void errorDispatch() {
+ getAdapter().errorDispatch(request, response);
+ }
+
+ @Override
public abstract boolean isComet();
@Override
Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/Adapter.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/Adapter.java?rev=1602510&r1=1602509&r2=1602510&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/coyote/Adapter.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/Adapter.java Fri Jun 13 20:21:32 2014
@@ -51,6 +51,8 @@ public interface Adapter {
public boolean asyncDispatch(Request req,Response res, SocketStatus status)
throws Exception;
+ public void errorDispatch(Request request, Response response);
+
public void log(Request req, Response res, long time);
/**
Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/LocalStrings.properties?rev=1602510&r1=1602509&r2=1602510&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/coyote/LocalStrings.properties (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/LocalStrings.properties Fri Jun 13 20:21:32 2014
@@ -16,6 +16,8 @@ abstractConnectionHandler.error=Error re
abstractConnectionHandler.ioexception.debug=IOExceptions are normal, ignored
abstractConnectionHandler.socketexception.debug=SocketExceptions are normal, ignored
+abstractProcessor.nonContainerThreadError=An error occurred in processing while on a non-container thread. The connection will be closed immediately
+
abstractProtocolHandler.getAttribute=Get attribute [{0}] with value [{1}]
abstractProtocolHandler.setAttribute=Set attribute [{0}] with value [{1}]
abstractProtocolHandler.init=Initializing ProtocolHandler [{0}]
Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/Processor.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/Processor.java?rev=1602510&r1=1602509&r2=1602510&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/coyote/Processor.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/Processor.java Fri Jun 13 20:21:32 2014
@@ -54,6 +54,8 @@ public interface Processor<S> {
HttpUpgradeHandler getHttpUpgradeHandler();
SocketState upgradeDispatch(SocketStatus status) throws IOException;
+ void errorDispatch();
+
boolean isComet();
boolean isAsync();
boolean isUpgrade();
Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java?rev=1602510&r1=1602509&r2=1602510&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java Fri Jun 13 20:21:32 2014
@@ -319,13 +319,13 @@ public abstract class AbstractAjpProcess
try {
prepareResponse();
} catch (IOException e) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
}
try {
flush(false);
} catch (IOException e) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
}
break;
}
@@ -335,7 +335,7 @@ public abstract class AbstractAjpProcess
try {
prepareResponse();
} catch (IOException e) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
return;
}
}
@@ -343,7 +343,7 @@ public abstract class AbstractAjpProcess
try {
flush(true);
} catch (IOException e) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
}
break;
}
@@ -354,7 +354,7 @@ public abstract class AbstractAjpProcess
case DISABLE_SWALLOW_INPUT: {
// TODO: Do not swallow request input but
// make sure we are closing the connection
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, null);
break;
}
case CLOSE: {
@@ -365,7 +365,7 @@ public abstract class AbstractAjpProcess
try {
finish();
} catch (IOException e) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
}
break;
}
@@ -494,7 +494,7 @@ public abstract class AbstractAjpProcess
case CLOSE_NOW: {
// Prevent further writes to the response
swallowResponse = true;
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, null);
break;
}
}
@@ -508,14 +508,14 @@ public abstract class AbstractAjpProcess
try {
rp.setStage(org.apache.coyote.Constants.STAGE_SERVICE);
if(!getAdapter().asyncDispatch(request, response, status)) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, null);
}
resetTimeouts();
} catch (InterruptedIOException e) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
} catch (Throwable t) {
ExceptionUtils.handleThrowable(t);
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, t);
getLog().error(sm.getString("http11processor.request.process"), t);
} finally {
if (getErrorState().isError()) {
@@ -761,7 +761,7 @@ public abstract class AbstractAjpProcess
long cl = vMB.getLong();
if (contentLengthSet) {
response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, null);
} else {
contentLengthSet = true;
// Set the content-length header for the request
@@ -879,7 +879,7 @@ public abstract class AbstractAjpProcess
secret = true;
if (!tmpMB.equals(requiredSecret)) {
response.setStatus(403);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, null);
}
}
break;
@@ -895,7 +895,7 @@ public abstract class AbstractAjpProcess
// Check if secret was submitted if required
if ((requiredSecret != null) && !secret) {
response.setStatus(403);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, null);
}
// Check for a full URI (including protocol://host:port/)
@@ -946,7 +946,7 @@ public abstract class AbstractAjpProcess
request.serverName().duplicate(request.localName());
} catch (IOException e) {
response.setStatus(400);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, e);
}
return;
}
@@ -996,7 +996,7 @@ public abstract class AbstractAjpProcess
// Invalid character
// 400 - Bad request
response.setStatus(400);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, null);
break;
}
port = port + (charValue * mult);
@@ -1112,7 +1112,7 @@ public abstract class AbstractAjpProcess
try {
prepareResponse();
} catch (IOException e) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
return;
}
}
@@ -1196,7 +1196,7 @@ public abstract class AbstractAjpProcess
try {
prepareResponse();
} catch (IOException e) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
}
}
Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java?rev=1602510&r1=1602509&r2=1602510&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java Fri Jun 13 20:21:32 2014
@@ -132,7 +132,7 @@ public class AjpAprProcessor extends Abs
cping = true;
if (Socket.send(socketRef, pongMessageArray, 0,
pongMessageArray.length) < 0) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, null);
}
continue;
} else if(type != Constants.JK_AJP13_FORWARD_REQUEST) {
@@ -141,20 +141,20 @@ public class AjpAprProcessor extends Abs
if (log.isDebugEnabled()) {
log.debug("Unexpected message: " + type);
}
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, null);
break;
}
keptAlive = true;
request.setStartTime(System.currentTimeMillis());
} catch (IOException e) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
break;
} catch (Throwable t) {
ExceptionUtils.handleThrowable(t);
log.debug(sm.getString("ajpprocessor.header.error"), t);
// 400 - Bad Request
response.setStatus(400);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, t);
getAdapter().log(request, response, 0);
}
@@ -168,7 +168,7 @@ public class AjpAprProcessor extends Abs
log.debug(sm.getString("ajpprocessor.request.prepare"), t);
// 500 - Internal Server Error
response.setStatus(500);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, t);
getAdapter().log(request, response, 0);
}
}
@@ -176,7 +176,7 @@ public class AjpAprProcessor extends Abs
if (!getErrorState().isError() && !cping && endpoint.isPaused()) {
// 503 - Service unavailable
response.setStatus(503);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, null);
getAdapter().log(request, response, 0);
}
cping = false;
@@ -187,13 +187,13 @@ public class AjpAprProcessor extends Abs
rp.setStage(org.apache.coyote.Constants.STAGE_SERVICE);
adapter.service(request, response);
} catch (InterruptedIOException e) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
} catch (Throwable t) {
ExceptionUtils.handleThrowable(t);
log.error(sm.getString("ajpprocessor.request.process"), t);
// 500 - Internal Server Error
response.setStatus(500);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, t);
getAdapter().log(request, response, 0);
}
}
@@ -208,7 +208,7 @@ public class AjpAprProcessor extends Abs
finish();
} catch (Throwable t) {
ExceptionUtils.handleThrowable(t);
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, t);
}
}
Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpNioProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpNioProcessor.java?rev=1602510&r1=1602509&r2=1602510&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpNioProcessor.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpNioProcessor.java Fri Jun 13 20:21:32 2014
@@ -118,7 +118,7 @@ public class AjpNioProcessor extends Abs
try {
output(pongMessageArray, 0, pongMessageArray.length);
} catch (IOException e) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, null);
}
recycle(false);
continue;
@@ -128,20 +128,20 @@ public class AjpNioProcessor extends Abs
if (log.isDebugEnabled()) {
log.debug("Unexpected message: " + type);
}
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, null);
recycle(true);
break;
}
request.setStartTime(System.currentTimeMillis());
} catch (IOException e) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
break;
} catch (Throwable t) {
ExceptionUtils.handleThrowable(t);
log.debug(sm.getString("ajpprocessor.header.error"), t);
// 400 - Bad Request
response.setStatus(400);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, t);
getAdapter().log(request, response, 0);
}
@@ -155,7 +155,7 @@ public class AjpNioProcessor extends Abs
log.debug(sm.getString("ajpprocessor.request.prepare"), t);
// 500 - Internal Server Error
response.setStatus(500);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, t);
getAdapter().log(request, response, 0);
}
}
@@ -163,7 +163,7 @@ public class AjpNioProcessor extends Abs
if (!getErrorState().isError() && !cping && endpoint.isPaused()) {
// 503 - Service unavailable
response.setStatus(503);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, null);
getAdapter().log(request, response, 0);
}
cping = false;
@@ -174,13 +174,13 @@ public class AjpNioProcessor extends Abs
rp.setStage(org.apache.coyote.Constants.STAGE_SERVICE);
adapter.service(request, response);
} catch (InterruptedIOException e) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
} catch (Throwable t) {
ExceptionUtils.handleThrowable(t);
log.error(sm.getString("ajpprocessor.request.process"), t);
// 500 - Internal Server Error
response.setStatus(500);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, t);
getAdapter().log(request, response, 0);
}
}
@@ -195,7 +195,7 @@ public class AjpNioProcessor extends Abs
finish();
} catch (Throwable t) {
ExceptionUtils.handleThrowable(t);
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, t);
}
}
Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java?rev=1602510&r1=1602509&r2=1602510&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java Fri Jun 13 20:21:32 2014
@@ -135,7 +135,7 @@ public class AjpProcessor extends Abstra
try {
output.write(pongMessageArray);
} catch (IOException e) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
}
continue;
} else if(type != Constants.JK_AJP13_FORWARD_REQUEST) {
@@ -144,19 +144,19 @@ public class AjpProcessor extends Abstra
if (log.isDebugEnabled()) {
log.debug("Unexpected message: " + type);
}
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, null);
break;
}
request.setStartTime(System.currentTimeMillis());
} catch (IOException e) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
break;
} catch (Throwable t) {
ExceptionUtils.handleThrowable(t);
log.debug(sm.getString("ajpprocessor.header.error"), t);
// 400 - Bad Request
response.setStatus(400);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, t);
getAdapter().log(request, response, 0);
}
@@ -170,7 +170,7 @@ public class AjpProcessor extends Abstra
log.debug(sm.getString("ajpprocessor.request.prepare"), t);
// 500 - Internal Server Error
response.setStatus(500);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, t);
getAdapter().log(request, response, 0);
}
}
@@ -178,7 +178,7 @@ public class AjpProcessor extends Abstra
if (!getErrorState().isError() && !cping && endpoint.isPaused()) {
// 503 - Service unavailable
response.setStatus(503);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, null);
getAdapter().log(request, response, 0);
}
cping = false;
@@ -189,13 +189,13 @@ public class AjpProcessor extends Abstra
rp.setStage(org.apache.coyote.Constants.STAGE_SERVICE);
adapter.service(request, response);
} catch (InterruptedIOException e) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
} catch (Throwable t) {
ExceptionUtils.handleThrowable(t);
log.error(sm.getString("ajpprocessor.request.process"), t);
// 500 - Internal Server Error
response.setStatus(500);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, t);
getAdapter().log(request, response, 0);
}
}
@@ -210,7 +210,7 @@ public class AjpProcessor extends Abstra
finish();
} catch (Throwable t) {
ExceptionUtils.handleThrowable(t);
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, t);
}
}
Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java?rev=1602510&r1=1602509&r2=1602510&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java Fri Jun 13 20:21:32 2014
@@ -734,7 +734,7 @@ public abstract class AbstractHttp11Proc
// Unsupported transfer encoding
// 501 - Unimplemented
response.setStatus(501);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, null);
if (getLog().isDebugEnabled()) {
getLog().debug(sm.getString("http11processor.request.prepare") +
" Unsupported transfer encoding [" + encodingName + "]");
@@ -759,7 +759,7 @@ public abstract class AbstractHttp11Proc
try {
getOutputBuffer().endRequest();
} catch (IOException e) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
}
break;
}
@@ -774,7 +774,7 @@ public abstract class AbstractHttp11Proc
prepareResponse();
getOutputBuffer().commit();
} catch (IOException e) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
}
break;
}
@@ -790,7 +790,7 @@ public abstract class AbstractHttp11Proc
try {
getOutputBuffer().sendAck();
} catch (IOException e) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
}
break;
}
@@ -798,7 +798,7 @@ public abstract class AbstractHttp11Proc
try {
getOutputBuffer().flush();
} catch (IOException e) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
response.setErrorException(e);
}
break;
@@ -810,7 +810,7 @@ public abstract class AbstractHttp11Proc
case DISABLE_SWALLOW_INPUT: {
// Do not swallow request input and make sure we are closing the
// connection
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, null);
getInputBuffer().setSwallowInput(false);
break;
}
@@ -894,7 +894,7 @@ public abstract class AbstractHttp11Proc
case CLOSE_NOW: {
// Block further output
getOutputBuffer().finished = true;
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, null);
break;
}
default: {
@@ -989,7 +989,7 @@ public abstract class AbstractHttp11Proc
if (endpoint.isPaused()) {
// 503 - Service unavailable
response.setStatus(503);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, null);
} else {
// Make sure that connectors that are non-blocking during
// header processing (NIO) only set the start time the first
@@ -1017,7 +1017,7 @@ public abstract class AbstractHttp11Proc
getLog().debug(
sm.getString("http11processor.header.parse"), e);
}
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
break;
} catch (Throwable t) {
ExceptionUtils.handleThrowable(t);
@@ -1039,7 +1039,7 @@ public abstract class AbstractHttp11Proc
}
// 400 - Bad Request
response.setStatus(400);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, t);
getAdapter().log(request, response, 0);
}
@@ -1056,7 +1056,7 @@ public abstract class AbstractHttp11Proc
}
// 500 - Internal Server Error
response.setStatus(500);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, t);
getAdapter().log(request, response, 0);
}
}
@@ -1082,20 +1082,20 @@ public abstract class AbstractHttp11Proc
response.getErrorException() != null ||
(!isAsync() &&
statusDropsConnection(response.getStatus())))) {
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, null);
}
setCometTimeouts(socketWrapper);
} catch (InterruptedIOException e) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
} catch (HeadersTooLargeException e) {
// The response should not have been committed but check it
// anyway to be safe
if (response.isCommitted()) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
} else {
response.reset();
response.setStatus(500);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, e);
response.setHeader("Connection", "close"); // TODO: Remove
}
} catch (Throwable t) {
@@ -1104,7 +1104,7 @@ public abstract class AbstractHttp11Proc
"http11processor.request.process"), t);
// 500 - Internal Server Error
response.setStatus(500);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, t);
getAdapter().log(request, response, 0);
}
}
@@ -1224,7 +1224,7 @@ public abstract class AbstractHttp11Proc
http11 = false;
// Send 505; Unsupported HTTP version
response.setStatus(505);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, null);
if (getLog().isDebugEnabled()) {
getLog().debug(sm.getString("http11processor.request.prepare")+
" Unsupported HTTP version \""+protocolMB+"\"");
@@ -1262,7 +1262,7 @@ public abstract class AbstractHttp11Proc
expectation = true;
} else {
response.setStatus(HttpServletResponse.SC_EXPECTATION_FAILED);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, null);
}
}
@@ -1356,7 +1356,7 @@ public abstract class AbstractHttp11Proc
if (http11 && (valueMB == null)) {
// 400 - Bad request
response.setStatus(400);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, null);
if (getLog().isDebugEnabled()) {
getLog().debug(sm.getString("http11processor.request.prepare")+
" host header missing");
@@ -1636,7 +1636,7 @@ public abstract class AbstractHttp11Proc
// Invalid character
// 400 - Bad request
response.setStatus(400);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, null);
break;
}
port = port + (charValue * mult);
@@ -1655,14 +1655,14 @@ public abstract class AbstractHttp11Proc
try {
rp.setStage(org.apache.coyote.Constants.STAGE_SERVICE);
if (!getAdapter().asyncDispatch(request, response, status)) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, null);
}
resetTimeouts();
} catch (InterruptedIOException e) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
} catch (Throwable t) {
ExceptionUtils.handleThrowable(t);
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, t);
getLog().error(sm.getString("http11processor.request.process"), t);
} finally {
if (getErrorState().isError()) {
@@ -1756,14 +1756,14 @@ public abstract class AbstractHttp11Proc
try {
getInputBuffer().endRequest();
} catch (IOException e) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
} catch (Throwable t) {
ExceptionUtils.handleThrowable(t);
// 500 - Internal Server Error
// Can't add a 500 to the access log since that has already been
// written in the Adapter.service method.
response.setStatus(500);
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, t);
getLog().error(sm.getString("http11processor.request.finish"), t);
}
}
@@ -1771,10 +1771,10 @@ public abstract class AbstractHttp11Proc
try {
getOutputBuffer().endRequest();
} catch (IOException e) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
} catch (Throwable t) {
ExceptionUtils.handleThrowable(t);
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, t);
getLog().error(sm.getString("http11processor.response.finish"), t);
}
}
Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java?rev=1602510&r1=1602509&r2=1602510&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java Fri Jun 13 20:21:32 2014
@@ -126,15 +126,15 @@ public class Http11AprProcessor extends
try {
rp.setStage(org.apache.coyote.Constants.STAGE_SERVICE);
if (!getAdapter().event(request, response, status)) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, null);
}
} catch (InterruptedIOException e) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
} catch (Throwable t) {
ExceptionUtils.handleThrowable(t);
// 500 - Internal Server Error
response.setStatus(500);
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, t);
getAdapter().log(request, response, 0);
log.error(sm.getString("http11processor.request.process"), t);
}
@@ -191,7 +191,7 @@ public class Http11AprProcessor extends
if (endpoint.isPaused()) {
// 503 - Service unavailable
response.setStatus(503);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, null);
getAdapter().log(request, response, 0);
} else {
return true;
@@ -228,7 +228,7 @@ public class Http11AprProcessor extends
log.debug(sm.getString(
"http11processor.sendfile.error"));
}
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, null);
} else {
// The sendfile Poller will add the socket to the main
// Poller once sendfile processing is complete
Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java?rev=1602510&r1=1602509&r2=1602510&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java Fri Jun 13 20:21:32 2014
@@ -116,7 +116,7 @@ public class Http11NioProcessor extends
try {
rp.setStage(org.apache.coyote.Constants.STAGE_SERVICE);
if (!getAdapter().event(request, response, status)) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, null);
}
if (!getErrorState().isError()) {
if (attach != null) {
@@ -139,12 +139,12 @@ public class Http11NioProcessor extends
}
}
} catch (InterruptedIOException e) {
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, e);
} catch (Throwable t) {
ExceptionUtils.handleThrowable(t);
// 500 - Internal Server Error
response.setStatus(500);
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, t);
log.error(sm.getString("http11processor.request.process"), t);
getAdapter().log(request, response, 0);
}
@@ -233,7 +233,7 @@ public class Http11NioProcessor extends
if (endpoint.isPaused()) {
// 503 - Service unavailable
response.setStatus(503);
- setErrorState(ErrorState.CLOSE_CLEAN);
+ setErrorState(ErrorState.CLOSE_CLEAN, null);
getAdapter().log(request, response, 0);
} else {
return true;
@@ -287,7 +287,7 @@ public class Http11NioProcessor extends
if (log.isDebugEnabled()) {
log.debug(sm.getString("http11processor.sendfile.error"));
}
- setErrorState(ErrorState.CLOSE_NOW);
+ setErrorState(ErrorState.CLOSE_NOW, null);
}
return true;
}
Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/upgrade/AbstractProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/upgrade/AbstractProcessor.java?rev=1602510&r1=1602509&r2=1602510&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/upgrade/AbstractProcessor.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/upgrade/AbstractProcessor.java Fri Jun 13 20:21:32 2014
@@ -161,6 +161,11 @@ public abstract class AbstractProcessor<
}
@Override
+ public void errorDispatch() {
+ // NO-OP
+ }
+
+ @Override
public final SocketState asyncPostProcess() {
return null;
}
Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/upgrade/UpgradeProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/upgrade/UpgradeProcessor.java?rev=1602510&r1=1602509&r2=1602510&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/upgrade/UpgradeProcessor.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/upgrade/UpgradeProcessor.java Fri Jun 13 20:21:32 2014
@@ -135,6 +135,11 @@ public abstract class UpgradeProcessor<S
}
@Override
+ public void errorDispatch() {
+ // NO-OP
+ }
+
+ @Override
public final SocketState asyncPostProcess() {
return null;
}
Modified: tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java?rev=1602510&r1=1602509&r2=1602510&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java Fri Jun 13 20:21:32 2014
@@ -623,7 +623,7 @@ public abstract class AbstractEndpoint<S
// ---------------------------------------------- Request processing methods
- protected abstract void processSocketAsync(SocketWrapper<S> socketWrapper,
+ public abstract void processSocketAsync(SocketWrapper<S> socketWrapper,
SocketStatus socketStatus);
Modified: tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/NioChannel.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/NioChannel.java?rev=1602510&r1=1602509&r2=1602510&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/NioChannel.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/NioChannel.java Fri Jun 13 20:21:32 2014
@@ -27,6 +27,7 @@ import java.nio.channels.SocketChannel;
import org.apache.tomcat.util.net.NioEndpoint.Poller;
import org.apache.tomcat.util.net.SecureNioChannel.ApplicationBufferHandler;
+import org.apache.tomcat.util.res.StringManager;
/**
*
@@ -39,6 +40,9 @@ import org.apache.tomcat.util.net.Secure
*/
public class NioChannel implements ByteChannel{
+ protected static final StringManager sm =
+ StringManager.getManager("org.apache.tomcat.util.net.res");
+
protected static ByteBuffer emptyBuf = ByteBuffer.allocate(0);
protected SocketChannel sc = null;
@@ -120,6 +124,7 @@ public class NioChannel implements ByteC
*/
@Override
public int write(ByteBuffer src) throws IOException {
+ checkInterruptStatus();
return sc.write(src);
}
@@ -224,4 +229,19 @@ public class NioChannel implements ByteC
}
+ /**
+ * This method should be used to check the interrupt status before
+ * attempting a write.
+ *
+ * If a thread has been interrupted and the interrupt has not been cleared
+ * then an attempt to write to the socket will fail. When this happens the
+ * socket is removed from the poller without the socket being selected. This
+ * results in a connection limit leak for NIO as the endpoint expects the
+ * socket to be selected even in error conditions.
+ */
+ protected void checkInterruptStatus() throws IOException {
+ if (Thread.interrupted()) {
+ throw new IOException(sm.getString("channel.nio.interrupted"));
+ }
+ }
}
Modified: tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java?rev=1602510&r1=1602509&r2=1602510&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java Fri Jun 13 20:21:32 2014
@@ -723,7 +723,7 @@ public class NioEndpoint extends Abstrac
@Override
- protected void processSocketAsync(SocketWrapper<NioChannel> socketWrapper,
+ public void processSocketAsync(SocketWrapper<NioChannel> socketWrapper,
SocketStatus socketStatus) {
dispatchForEvent(socketWrapper.getSocket(), socketStatus, true);
}
Modified: tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java?rev=1602510&r1=1602509&r2=1602510&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java Fri Jun 13 20:21:32 2014
@@ -471,6 +471,7 @@ public class SecureNioChannel extends Ni
*/
@Override
public int write(ByteBuffer src) throws IOException {
+ checkInterruptStatus();
if ( src == this.netOutBuffer ) {
//we can get here through a recursive call
//by using the NioBlockingSelector
Modified: tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/SocketStatus.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/SocketStatus.java?rev=1602510&r1=1602509&r2=1602510&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/SocketStatus.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/SocketStatus.java Fri Jun 13 20:21:32 2014
@@ -23,5 +23,5 @@ package org.apache.tomcat.util.net;
* @author remm
*/
public enum SocketStatus {
- OPEN_READ, OPEN_WRITE, STOP, TIMEOUT, DISCONNECT, ERROR
+ OPEN_READ, OPEN_WRITE, STOP, TIMEOUT, DISCONNECT, ERROR, CLOSE_NOW
}
Modified: tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/res/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/res/LocalStrings.properties?rev=1602510&r1=1602509&r2=1602510&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/res/LocalStrings.properties (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/res/LocalStrings.properties Fri Jun 13 20:21:32 2014
@@ -57,3 +57,5 @@ endpoint.apr.pollUnknownEvent=A socket w
endpoint.apr.remoteport=APR socket [{0}] opened with remote port [{1}]
endpoint.nio.selectorCloseFail=Failed to close selector when closing the poller
endpoint.warn.noExector=Failed to process socket [{0}] in state [{1}] because the executor had already been shutdown
+
+channel.nio.interrupted=The current thread was interrupted
Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1602510&r1=1602509&r2=1602510&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Fri Jun 13 20:21:32 2014
@@ -106,6 +106,12 @@
<subsection name="Coyote">
<changelog>
<fix>
+ <bug>56518</bug>: When using NIO, do not attempt to write to the socket
+ if the thread is marked interrupted as this will lead to a connection
+ limit leak. This fix was based on analysis of the issue by hanyong.
+ (markt)
+ </fix>
+ <fix>
<bug>56521</bug>: Re-use the asynchronous write buffer between writes to
reduce allocation and GC overhead. Based on a patch by leonzhx. Also
make the buffer size configurable and remove copying of data within
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1602510 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/catalina/connector/
java/org/apache/coyote/ java/org/apache/coyote/ajp/ java/org/apache/coyote/http11/
java/org/apache/coyote/http11/upgrade/ java/org/apache/tomcat/util/net/ java/org/...
Posted by Mark Thomas <ma...@apache.org>.
On 18/06/2014 13:06, Konstantin Kolinko wrote:
> 2014-06-17 17:08 GMT+04:00 Mark Thomas <ma...@apache.org>:
>> On 17/06/2014 00:50, Konstantin Kolinko wrote:
>>> 2014-06-14 0:21 GMT+04:00 <ma...@apache.org>:
>>>> Author: markt
>>>> Date: Fri Jun 13 20:21:32 2014
>>>> New Revision: 1602510
>>
>> <snip/>
>>
>>> 3) In case if there are several executor threads that process the
>>> socket at the same time, what happens?
>>>
>>> My understanding/answer is that "synchronized (socket)" in
>>> NioEndpoint$SocketProcessor.run() handles that.
>>
>> Correct. You don't want multiple threads trying to read from the socket
>> at the same time. Neither do you want multiple threads trying to writ to
>> the socket at the same time. One thread reading and one thread writing
>> is OK.
>>
>>> In this case I see that NioEndpoint$SocketProcessor.doRun() does
>>> "nioChannels.offer(socket)"
>>> That is it offers the "socket" for reuse.
>>>
>>> Thus "synchronized (socket)" is broken, as the "socket" can be
>>> recycled and reused by another thread while the second one waits to
>>> obtain the monitor on the "socket".
>>
>> In theory yes. In practise, I don't believe that this will happen.
>>
>> The syncs were added as part of the Servlet 3.0 async processing
>> implementation. The case they were trying to deal with was:
>> a) app on container thread starts async processing
>> b) app passes control to app thread
>> c) container thread is returned to container for reuse
>> (update state, does a little clean up before it is reused)
>> d) app thread completes the async processing
>> e) control returns to container thread to complete the request
>>
>> There was a chance that e) could complete before c) and that caused all
>> sorts of confusion. The syncs ensure c) completes before e) starts.
>>
>> For the problem you describe to occur a fatal I/O error would have to
>> occur during c) after e) had reached the sync. Since there should be no
>> I/O during c) I don't think this can happen.
>>
>> If you have a scenario where you think this is an issue, please share it.
>
> I suspect that the following scenario might happen:
> a),b),c) the same as above
>
> d) App thread performs I/O and an error happens
> (e.g. in 'COMMIT' or 'ACK' case in AbstractHttp11Processor.action())
> e) AbstractProcessor.setErrorState() dispatches SocketStatus.CLOSE_NOW
>
> f) App thread calls async.complete() or async.dispatch()
> g) AbstractHttp11Processor.action() dispatches SocketStatus.OPEN_READ
>
> How the race between e) and g) is resolved?
I don't think it is at the moment. I'm not sure if the spec covers what
should happen in this case. For example, should the app continue and
call complete() or dispatch()? Should the onError() method of any Async
listener be called?
One thing I am fairly sure of, the more we can refactor the connectors
to reduce duplication, the easier handling this sort of thing will
become. That refactoring is moving up my TODO list for Tomcat 9 the more
I think about it.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1602510 - in /tomcat/tc7.0.x/trunk: ./
java/org/apache/catalina/connector/ java/org/apache/coyote/
java/org/apache/coyote/ajp/ java/org/apache/coyote/http11/
java/org/apache/coyote/http11/upgrade/ java/org/apache/tomcat/util/net/ java/org/...
Posted by Konstantin Kolinko <kn...@gmail.com>.
2014-06-17 17:08 GMT+04:00 Mark Thomas <ma...@apache.org>:
> On 17/06/2014 00:50, Konstantin Kolinko wrote:
>> 2014-06-14 0:21 GMT+04:00 <ma...@apache.org>:
>>> Author: markt
>>> Date: Fri Jun 13 20:21:32 2014
>>> New Revision: 1602510
>
> <snip/>
>
>> 3) In case if there are several executor threads that process the
>> socket at the same time, what happens?
>>
>> My understanding/answer is that "synchronized (socket)" in
>> NioEndpoint$SocketProcessor.run() handles that.
>
> Correct. You don't want multiple threads trying to read from the socket
> at the same time. Neither do you want multiple threads trying to writ to
> the socket at the same time. One thread reading and one thread writing
> is OK.
>
>> In this case I see that NioEndpoint$SocketProcessor.doRun() does
>> "nioChannels.offer(socket)"
>> That is it offers the "socket" for reuse.
>>
>> Thus "synchronized (socket)" is broken, as the "socket" can be
>> recycled and reused by another thread while the second one waits to
>> obtain the monitor on the "socket".
>
> In theory yes. In practise, I don't believe that this will happen.
>
> The syncs were added as part of the Servlet 3.0 async processing
> implementation. The case they were trying to deal with was:
> a) app on container thread starts async processing
> b) app passes control to app thread
> c) container thread is returned to container for reuse
> (update state, does a little clean up before it is reused)
> d) app thread completes the async processing
> e) control returns to container thread to complete the request
>
> There was a chance that e) could complete before c) and that caused all
> sorts of confusion. The syncs ensure c) completes before e) starts.
>
> For the problem you describe to occur a fatal I/O error would have to
> occur during c) after e) had reached the sync. Since there should be no
> I/O during c) I don't think this can happen.
>
> If you have a scenario where you think this is an issue, please share it.
I suspect that the following scenario might happen:
a),b),c) the same as above
d) App thread performs I/O and an error happens
(e.g. in 'COMMIT' or 'ACK' case in AbstractHttp11Processor.action())
e) AbstractProcessor.setErrorState() dispatches SocketStatus.CLOSE_NOW
f) App thread calls async.complete() or async.dispatch()
g) AbstractHttp11Processor.action() dispatches SocketStatus.OPEN_READ
How the race between e) and g) is resolved?
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1602510 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/catalina/connector/
java/org/apache/coyote/ java/org/apache/coyote/ajp/ java/org/apache/coyote/http11/
java/org/apache/coyote/http11/upgrade/ java/org/apache/tomcat/util/net/ java/org/...
Posted by Mark Thomas <ma...@apache.org>.
On 17/06/2014 17:09, Konstantin Kolinko wrote:
<snip/>
> In Tomcat 8 there was the following commit as part of fixing this issue:
> http://svn.apache.org/viewvc?view=revision&revision=r1594411
>
> Are there plans to backport it to Tomcat 7?
Not from me.
It looks like that patch just changed the timing so the issue was harder
to re-create so it didn't actually fix anything. Again no objections if
someone else wants to take a look.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1602510 - in /tomcat/tc7.0.x/trunk: ./
java/org/apache/catalina/connector/ java/org/apache/coyote/
java/org/apache/coyote/ajp/ java/org/apache/coyote/http11/
java/org/apache/coyote/http11/upgrade/ java/org/apache/tomcat/util/net/ java/org/...
Posted by Konstantin Kolinko <kn...@gmail.com>.
2014-06-17 17:08 GMT+04:00 Mark Thomas <ma...@apache.org>:
> On 17/06/2014 00:50, Konstantin Kolinko wrote:
>> 2014-06-14 0:21 GMT+04:00 <ma...@apache.org>:
>>> Author: markt
>>> Date: Fri Jun 13 20:21:32 2014
>>> New Revision: 1602510
> <snip/>
>> 2) The submission of SocketProcessor to the Executor may end with
>> RejectedExecutionException.
>> - in NioEndpoint#processSocket(NioChannel, SocketStatus, boolean)
>>
>> In that case it just logs a warning and does nothing else.
>>
>> How likely is this rejected execution?
>
> Very unlikely.
>
>> My understanding/answer is that the rejection is unlikely.
>> The executor here is o.a.c.core.StandardThreadExecutor. Its execute()
>> method is unlikely to reject tasks. There is a queue that accepts
>> tasks. Tasks will be rejected if either
>> a) Executor is not running.
>> b) Queue is full. The maximum queue length is Integer.MAX_VALUE by
>> default. Though its size is configurable via
>> StandardThreadExecutor.setMaxQueueSize().
>>
>> Will it result in failing to decrement the connection counter?
>> Will it result in failing to obtain and to recycle the processor
>> associated with the socket?
>>
>> I suspect that per this BZ 56518 it will fail to decrement the counter.
>
> The only time I think this might occur is during connector stop() in
> which case the socket will be closed and the connection counter reset
> anyway.
>
ACK.
Maybe I'll add a warning to the docs to discourage changing maxQueueSize.
<snip/> ....
>
> If you have a scenario where you think this is an issue, please share it.
>
>> 4) There are no tests for this new feature.
>
> I have been using the test WAR provided by the OP. I haven't had a
> chance to convert it to a unit test. Obviously, no objections from me if
> you want to do that.
Ack. Thanks for the pointer. (I am not sure that I'll find the time,
but now I know where to look).
In Tomcat 8 there was the following commit as part of fixing this issue:
http://svn.apache.org/viewvc?view=revision&revision=r1594411
Are there plans to backport it to Tomcat 7?
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1602510 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/catalina/connector/
java/org/apache/coyote/ java/org/apache/coyote/ajp/ java/org/apache/coyote/http11/
java/org/apache/coyote/http11/upgrade/ java/org/apache/tomcat/util/net/ java/org/...
Posted by Mark Thomas <ma...@apache.org>.
On 17/06/2014 00:50, Konstantin Kolinko wrote:
> 2014-06-14 0:21 GMT+04:00 <ma...@apache.org>:
>> Author: markt
>> Date: Fri Jun 13 20:21:32 2014
>> New Revision: 1602510
<snip/>
> I have several questions.
>
> 1) The AbstractProtocol$AbstractConnectionHandler.process() mentioned
> above is from Tomcat 8. The code for handling SocketStatus.CLOSE_NOW
> there is missing in backport to Tomcat 7.
>
> In Tomcat 7 the SocketStatus.CLOSE_NOW enum value is used only in 1
> place, where in Tomcat 8 it is used in 2 places..
It looks like I messed up the merge of that part of the back-port. Fixed
in r1603152.
> 2) The submission of SocketProcessor to the Executor may end with
> RejectedExecutionException.
> - in NioEndpoint#processSocket(NioChannel, SocketStatus, boolean)
>
> In that case it just logs a warning and does nothing else.
>
> How likely is this rejected execution?
Very unlikely.
> My understanding/answer is that the rejection is unlikely.
> The executor here is o.a.c.core.StandardThreadExecutor. Its execute()
> method is unlikely to reject tasks. There is a queue that accepts
> tasks. Tasks will be rejected if either
> a) Executor is not running.
> b) Queue is full. The maximum queue length is Integer.MAX_VALUE by
> default. Though its size is configurable via
> StandardThreadExecutor.setMaxQueueSize().
>
> Will it result in failing to decrement the connection counter?
> Will it result in failing to obtain and to recycle the processor
> associated with the socket?
>
> I suspect that per this BZ 56518 it will fail to decrement the counter.
The only time I think this might occur is during connector stop() in
which case the socket will be closed and the connection counter reset
anyway.
> 3) In case if there are several executor threads that process the
> socket at the same time, what happens?
>
> My understanding/answer is that "synchronized (socket)" in
> NioEndpoint$SocketProcessor.run() handles that.
Correct. You don't want multiple threads trying to read from the socket
at the same time. Neither do you want multiple threads trying to writ to
the socket at the same time. One thread reading and one thread writing
is OK.
> In this case I see that NioEndpoint$SocketProcessor.doRun() does
> "nioChannels.offer(socket)"
> That is it offers the "socket" for reuse.
>
> Thus "synchronized (socket)" is broken, as the "socket" can be
> recycled and reused by another thread while the second one waits to
> obtain the monitor on the "socket".
In theory yes. In practise, I don't believe that this will happen.
The syncs were added as part of the Servlet 3.0 async processing
implementation. The case they were trying to deal with was:
a) app on container thread starts async processing
b) app passes control to app thread
c) container thread is returned to container for reuse
(update state, does a little clean up before it is reused)
d) app thread completes the async processing
e) control returns to container thread to complete the request
There was a chance that e) could complete before c) and that caused all
sorts of confusion. The syncs ensure c) completes before e) starts.
For the problem you describe to occur a fatal I/O error would have to
occur during c) after e) had reached the sync. Since there should be no
I/O during c) I don't think this can happen.
If you have a scenario where you think this is an issue, please share it.
> 4) There are no tests for this new feature.
I have been using the test WAR provided by the OP. I haven't had a
chance to convert it to a unit test. Obviously, no objections from me if
you want to do that.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1602510 - in /tomcat/tc7.0.x/trunk: ./
java/org/apache/catalina/connector/ java/org/apache/coyote/
java/org/apache/coyote/ajp/ java/org/apache/coyote/http11/
java/org/apache/coyote/http11/upgrade/ java/org/apache/tomcat/util/net/ java/org/...
Posted by Konstantin Kolinko <kn...@gmail.com>.
2014-06-14 0:21 GMT+04:00 <ma...@apache.org>:
> Author: markt
> Date: Fri Jun 13 20:21:32 2014
> New Revision: 1602510
>
> URL: http://svn.apache.org/r1602510
> Log:
> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56518
> Do not attempt an NIO write if a thread has been interrupted as it can lead to a connection limit leak
>
> Modified:
> tomcat/tc7.0.x/trunk/ (props changed)
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
> tomcat/tc7.0.x/trunk/java/org/apache/coyote/AbstractProcessor.java
> tomcat/tc7.0.x/trunk/java/org/apache/coyote/Adapter.java
> tomcat/tc7.0.x/trunk/java/org/apache/coyote/LocalStrings.properties
> tomcat/tc7.0.x/trunk/java/org/apache/coyote/Processor.java
> tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java
> tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java
> tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpNioProcessor.java
> tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java
> tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
> tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java
> tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
> tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/upgrade/AbstractProcessor.java
> tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/upgrade/UpgradeProcessor.java
> tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java
> tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/NioChannel.java
> tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java
> tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java
> tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/SocketStatus.java
> tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/res/LocalStrings.properties
> tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
>
> Propchange: tomcat/tc7.0.x/trunk/
> ------------------------------------------------------------------------------
> Merged /tomcat/trunk:r1602198
>
> Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
> URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java?rev=1602510&r1=1602509&r2=1602510&view=diff
> ==============================================================================
> --- tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java (original)
> +++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java Fri Jun 13 20:21:32 2014
> @@ -469,6 +469,34 @@ public class CoyoteAdapter implements Ad
>
>
> @Override
> + public void errorDispatch(org.apache.coyote.Request req,
> + org.apache.coyote.Response res) {
> + Request request = (Request) req.getNote(ADAPTER_NOTES);
> + Response response = (Response) res.getNote(ADAPTER_NOTES);
> +
> + if (request != null && request.getMappingData().context != null) {
> + ((Context) request.getMappingData().context).logAccess(
> + request, response,
> + System.currentTimeMillis() - req.getStartTime(),
> + false);
> + } else {
> + log(req, res, System.currentTimeMillis() - req.getStartTime());
> + }
> +
> + if (request != null) {
> + request.recycle();
> + }
> +
> + if (response != null) {
> + response.recycle();
> + }
> +
> + res.recycle();
> + res.recycle();
I fixed the typo by r1602852.
> + }
In short, my summary on how this works (on example of NIO) on Tomcat 7
codebase is as following
In AbstractProcessor.setErrorState it now does
[[[
getEndpoint().processSocketAsync(socketWrapper, SocketStatus.CLOSE_NOW);
]]]
That goes (for example) into NioEndpoint.processSocketAsync(...) and
into NioEndpoint.processSocket(...) where it creates a new
SocketProcessor(socket, status) and submits it to an Executor.
NioEndpoint$SocketProcessor.run() obtains monitor on the socket (by
"synchronized (socket)"), calls processor (by "handler.process(ka,
status)"), receives SocketState.CLOSED from the call and counts down
the connection (done in getPoller().cancelledKey()).
The method "handler.process()" that is called there is
AbstractProtocol$AbstractConnectionHandler.process(SocketWrapper,
SocketStatus).
In Tomcat 8 it gets a processor for this socket (if there is one
associated with it), calls "processor.errorDispatch(...);" (the new
"CoyoteAdapter. errorDispatch(...)" method above) and recycles the
processor.
I have several questions.
1) The AbstractProtocol$AbstractConnectionHandler.process() mentioned
above is from Tomcat 8. The code for handling SocketStatus.CLOSE_NOW
there is missing in backport to Tomcat 7.
In Tomcat 7 the SocketStatus.CLOSE_NOW enum value is used only in 1
place, where in Tomcat 8 it is used in 2 places..
2) The submission of SocketProcessor to the Executor may end with
RejectedExecutionException.
- in NioEndpoint#processSocket(NioChannel, SocketStatus, boolean)
In that case it just logs a warning and does nothing else.
How likely is this rejected execution?
My understanding/answer is that the rejection is unlikely.
The executor here is o.a.c.core.StandardThreadExecutor. Its execute()
method is unlikely to reject tasks. There is a queue that accepts
tasks. Tasks will be rejected if either
a) Executor is not running.
b) Queue is full. The maximum queue length is Integer.MAX_VALUE by
default. Though its size is configurable via
StandardThreadExecutor.setMaxQueueSize().
Will it result in failing to decrement the connection counter?
Will it result in failing to obtain and to recycle the processor
associated with the socket?
I suspect that per this BZ 56518 it will fail to decrement the counter.
3) In case if there are several executor threads that process the
socket at the same time, what happens?
My understanding/answer is that "synchronized (socket)" in
NioEndpoint$SocketProcessor.run() handles that.
In this case I see that NioEndpoint$SocketProcessor.doRun() does
"nioChannels.offer(socket)"
That is it offers the "socket" for reuse.
Thus "synchronized (socket)" is broken, as the "socket" can be
recycled and reused by another thread while the second one waits to
obtain the monitor on the "socket".
4) There are no tests for this new feature.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org