Author: markt
Date: Tue May 13 22:51:19 2014
New Revision: 1594411
URL: http://svn.apache.org/r1594411
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56518
Refactor NIO's SocketProcessor to use KeyAtrtachment rather than Socket to
align it with the other implementations. This enables a number of
attachment->socket->attachment transitions to be skipped which appears to
resolve the root cause of BZ 56518. Since the wrapper is available and can be
processed, it becomes possible to reduce the connection count when the socket
is closed by the interrupt.
Modified:
tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java
tomcat/trunk/webapps/docs/changelog.xml
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java?rev=1594411&r1=1594410&r2=1594411&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java Tue May 13
22:51:19 2014
@@ -605,23 +605,22 @@ public class NioEndpoint extends Abstrac
public void processSocket(SocketWrapper<NioChannel> socketWrapper,
SocketStatus socketStatus, boolean dispatch) {
NioChannel socket = socketWrapper.getSocket();
- if (dispatch && socketStatus == SocketStatus.OPEN_READ) {
+ if (socket.isOpen() && dispatch && socketStatus ==
SocketStatus.OPEN_READ) {
socket.getPoller().add(socket, OP_CALLBACK);
} else {
- processSocket(socket, socketStatus, dispatch);
+ processSocket((KeyAttachment) socketWrapper, socketStatus,
dispatch);
}
}
- protected boolean processSocket(NioChannel socket, SocketStatus status,
boolean dispatch) {
+ protected boolean processSocket(KeyAttachment attachment, SocketStatus
status, boolean dispatch) {
try {
- KeyAttachment attachment =
(KeyAttachment)socket.getAttachment(false);
if (attachment == null) {
return false;
}
attachment.setCometNotify(false); //will get reset upon next reg
SocketProcessor sc = processorCache.pop();
- if ( sc == null ) sc = new SocketProcessor(socket,status);
- else sc.reset(socket,status);
+ if ( sc == null ) sc = new SocketProcessor(attachment, status);
+ else sc.reset(attachment, status);
Executor executor = getExecutor();
if (dispatch && executor != null) {
executor.execute(sc);
@@ -629,7 +628,7 @@ public class NioEndpoint extends Abstrac
sc.run();
}
} catch (RejectedExecutionException ree) {
- log.warn(sm.getString("endpoint.executor.fail", socket), ree);
+ log.warn(sm.getString("endpoint.executor.fail",
attachment.getSocket()), ree);
return false;
} catch (Throwable t) {
ExceptionUtils.handleThrowable(t);
@@ -900,7 +899,8 @@ public class NioEndpoint extends Abstrac
}
addEvent(r);
if (close) {
- processSocket(socket, SocketStatus.STOP, false);
+ NioEndpoint.KeyAttachment ka =
(NioEndpoint.KeyAttachment)socket.getAttachment(false);
+ processSocket(ka, SocketStatus.STOP, false);
}
}
@@ -956,12 +956,12 @@ public class NioEndpoint extends Abstrac
if (ka != null && ka.isComet() && status != null) {
ka.setComet(false);//to avoid a loop
if (status == SocketStatus.TIMEOUT ) {
- if (processSocket(ka.getSocket(), status, true)) {
+ if (processSocket(ka, status, true)) {
return; // don't close on comet timeout
}
} else {
// Don't dispatch if the lines below are canceling the
key
- processSocket(ka.getSocket(), status, false);
+ processSocket(ka, status, false);
}
}
key.attach(null);
@@ -1126,7 +1126,6 @@ public class NioEndpoint extends Abstrac
cancelledKey(sk, SocketStatus.STOP);
} else if ( sk.isValid() && attachment != null ) {
attachment.access();//make sure we don't time out valid
sockets
- NioChannel channel = attachment.getSocket();
if (sk.isReadable() || sk.isWritable() ) {
if ( attachment.getSendfileData() != null ) {
processSendfile(sk,attachment, false);
@@ -1136,12 +1135,12 @@ public class NioEndpoint extends Abstrac
boolean closeSocket = false;
// Read goes before write
if (sk.isReadable()) {
- if (!processSocket(channel,
SocketStatus.OPEN_READ, true)) {
+ if (!processSocket(attachment,
SocketStatus.OPEN_READ, true)) {
closeSocket = true;
}
}
if (!closeSocket && sk.isWritable()) {
- if (!processSocket(channel,
SocketStatus.OPEN_WRITE, true)) {
+ if (!processSocket(attachment,
SocketStatus.OPEN_WRITE, true)) {
closeSocket = true;
}
}
@@ -1302,7 +1301,7 @@ public class NioEndpoint extends Abstrac
int ops = ka.interestOps() & ~OP_CALLBACK;
reg(key,ka,0);//avoid multiple calls, this gets
re-registered after invocation
ka.interestOps(ops);
- if (!processSocket(ka.getSocket(),
SocketStatus.OPEN_READ, true)) processSocket(ka.getSocket(),
SocketStatus.DISCONNECT, true);
+ if (!processSocket(ka, SocketStatus.OPEN_READ, true))
processSocket(ka, SocketStatus.DISCONNECT, true);
} else if ((ka.interestOps()&SelectionKey.OP_READ) ==
SelectionKey.OP_READ ||
(ka.interestOps()&SelectionKey.OP_WRITE) ==
SelectionKey.OP_WRITE) {
//only timeout sockets that we are waiting for a read
from
@@ -1331,7 +1330,7 @@ public class NioEndpoint extends Abstrac
if (isTimedout) {
// Prevent subsequent timeouts if the timeout
event takes a while to process
ka.access(Long.MAX_VALUE);
- processSocket(ka.getSocket(),
SocketStatus.TIMEOUT, true);
+ processSocket(ka, SocketStatus.TIMEOUT, true);
}
}
}//end if
@@ -1493,27 +1492,23 @@ public class NioEndpoint extends Abstrac
*/
protected class SocketProcessor implements Runnable {
- private NioChannel socket = null;
+ private KeyAttachment ka = null;
private SocketStatus status = null;
- public SocketProcessor(NioChannel socket, SocketStatus status) {
- reset(socket,status);
+ public SocketProcessor(KeyAttachment ka, SocketStatus status) {
+ reset(ka, status);
}
- public void reset(NioChannel socket, SocketStatus status) {
- this.socket = socket;
+ public void reset(KeyAttachment ka, SocketStatus status) {
+ this.ka = ka;
this.status = status;
}
@Override
public void run() {
+ NioChannel socket = ka.getSocket();
SelectionKey key = socket.getIOChannel().keyFor(
socket.getPoller().getSelector());
- KeyAttachment ka = null;
-
- if (key != null) {
- ka = (KeyAttachment)key.attachment();
- }
// Upgraded connections need to allow multiple threads to access
the
// connection at the same time to enable blocking IO to be used
when
@@ -1531,6 +1526,8 @@ public class NioEndpoint extends Abstrac
}
private void doRun(SelectionKey key, KeyAttachment ka) {
+ NioChannel socket = ka.getSocket();
+
try {
int handshake = -1;
@@ -1571,20 +1568,20 @@ public class NioEndpoint extends Abstrac
if (state == SocketState.CLOSED) {
// Close socket and pool
try {
- if (ka!=null) ka.setComet(false);
+ ka.setComet(false);
socket.getPoller().cancelledKey(key,
SocketStatus.ERROR);
if (running && !paused) {
nioChannels.push(socket);
}
socket = null;
- if (running && !paused && ka != null) {
+ if (running && !paused) {
keyCache.push(ka);
}
ka = null;
} catch (Exception x) {
log.error("",x);
}
- } else if (state == SocketState.LONG && ka != null &&
ka.isAsync() && ka.interestOps() > 0) {
+ } else if (state == SocketState.LONG && ka.isAsync() &&
ka.interestOps() > 0) {
//we are async, and we are interested in operations
ka.getPoller().add(socket, ka.interestOps());
}
@@ -1596,7 +1593,7 @@ public class NioEndpoint extends Abstrac
nioChannels.push(socket);
}
socket = null;
- if (running && !paused && ka != null) {
+ if (running && !paused) {
keyCache.push(ka);
}
ka = null;
@@ -1604,7 +1601,9 @@ public class NioEndpoint extends Abstrac
ka.getPoller().add(socket,handshake);
}
} catch (CancelledKeyException cx) {
- socket.getPoller().cancelledKey(key, null);
+ if (socket != null) {
+ socket.getPoller().cancelledKey(key, null);
+ }
} catch (OutOfMemoryError oom) {
try {
oomParachuteData = null;
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1594411&r1=1594410&r2=1594411&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Tue May 13 22:51:19 2014
@@ -232,6 +232,10 @@
<fix>
Fix possible corruption if doing keepalive after a comet request.
(remm)
</fix>
+ <fix>
+ <bug>56518</bug>: Fix connection limit latch leak when a non-container
+ thread is interrupted during asynchronous processing. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
