Author: markt Date: Tue Sep 17 09:44:32 2013 New Revision: 1523964 URL: http://svn.apache.org/r1523964 Log: Improve handling of situation where socket / connector closes down while an application thread is using the socket. This ismost likely to occur with upgraded connections that use concurrent read/write (e.g. WebSocket)
Modified: tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java tomcat/trunk/java/org/apache/coyote/http11/Http11AprProtocol.java tomcat/trunk/java/org/apache/coyote/http11/upgrade/AprServletInputStream.java tomcat/trunk/java/org/apache/coyote/http11/upgrade/AprServletOutputStream.java tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapper.java tomcat/trunk/java/org/apache/tomcat/util/net/res/LocalStrings.properties Modified: tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java?rev=1523964&r1=1523963&r2=1523964&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java (original) +++ tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java Tue Sep 17 09:44:32 2013 @@ -703,6 +703,7 @@ public abstract class AbstractProtocol<S } else { // Connection closed. OK to recycle the processor. Upgrade // processors are not recycled. + wrapper.setClosing(true); connections.remove(socket); if (processor.isUpgrade()) { processor.getHttpUpgradeHandler().destroy(); Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11AprProtocol.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11AprProtocol.java?rev=1523964&r1=1523963&r2=1523964&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/Http11AprProtocol.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/Http11AprProtocol.java Tue Sep 17 09:44:32 2013 @@ -28,6 +28,7 @@ import org.apache.juli.logging.LogFactor import org.apache.tomcat.util.net.AbstractEndpoint; import org.apache.tomcat.util.net.AprEndpoint; import org.apache.tomcat.util.net.AprEndpoint.Handler; +import org.apache.tomcat.util.net.AprEndpoint.Poller; import org.apache.tomcat.util.net.SocketStatus; import org.apache.tomcat.util.net.SocketWrapper; @@ -309,8 +310,13 @@ public class Http11AprProtocol extends A } } else { // Upgraded - ((AprEndpoint) proto.endpoint).getPoller().add( - socket.getSocket().longValue(), -1, true, false); + Poller p = ((AprEndpoint) proto.endpoint).getPoller(); + if (p == null) { + // Connector has been stopped + release(socket, processor, true, false); + } else { + p.add(socket.getSocket().longValue(), -1, true, false); + } } } Modified: tomcat/trunk/java/org/apache/coyote/http11/upgrade/AprServletInputStream.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/upgrade/AprServletInputStream.java?rev=1523964&r1=1523963&r2=1523964&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/upgrade/AprServletInputStream.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/upgrade/AprServletInputStream.java Tue Sep 17 09:44:32 2013 @@ -50,7 +50,7 @@ public class AprServletInputStream exten try { readLock.lock(); if (wrapper.getBlockingStatus() == block) { - if (closed) { + if (closed || wrapper.isClosing()) { throw new IOException(sm.getString("apr.closed")); } result = Socket.recv(socket, b, off, len); @@ -70,7 +70,7 @@ public class AprServletInputStream exten try { readLock.lock(); writeLock.unlock(); - if (closed) { + if (closed || wrapper.isClosing()) { throw new IOException(sm.getString("apr.closed")); } result = Socket.recv(socket, b, off, len); Modified: tomcat/trunk/java/org/apache/coyote/http11/upgrade/AprServletOutputStream.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/upgrade/AprServletOutputStream.java?rev=1523964&r1=1523963&r2=1523964&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/upgrade/AprServletOutputStream.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/upgrade/AprServletOutputStream.java Tue Sep 17 09:44:32 2013 @@ -60,7 +60,7 @@ public class AprServletOutputStream exte try { readLock.lock(); if (wrapper.getBlockingStatus() == block) { - if (closed) { + if (closed || wrapper.isClosing()) { throw new IOException(sm.getString("apr.closed")); } return doWriteInternal(b, off, len); @@ -83,7 +83,7 @@ public class AprServletOutputStream exte try { readLock.lock(); writeLock.unlock(); - if (closed) { + if (closed || wrapper.isClosing()) { throw new IOException(sm.getString("apr.closed")); } return doWriteInternal(b, off, len); Modified: tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java?rev=1523964&r1=1523963&r2=1523964&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java Tue Sep 17 09:44:32 2013 @@ -519,6 +519,18 @@ public abstract class AbstractEndpoint<S //this is our internal one, so we need to shut it down ThreadPoolExecutor tpe = (ThreadPoolExecutor) executor; tpe.shutdownNow(); + int count = 0; + while (count < 50 && tpe.isTerminating()) { + try { + Thread.sleep(100); + count++; + } catch (InterruptedException e) { + // Ignore + } + } + if (tpe.isTerminating()) { + getLog().warn(sm.getString("endpoint.warn.executorShutdown", getName())); + } TaskQueue queue = (TaskQueue) tpe.getQueue(); queue.setParent(null); } Modified: tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java?rev=1523964&r1=1523963&r2=1523964&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java Tue Sep 17 09:44:32 2013 @@ -1593,6 +1593,10 @@ public class AprEndpoint extends Abstrac } } + if (!pollerRunning) { + break; + } + try { // Add sockets which are waiting to the poller if (addList.size() > 0) { Modified: tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapper.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapper.java?rev=1523964&r1=1523963&r2=1523964&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapper.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapper.java Tue Sep 17 09:44:32 2013 @@ -64,6 +64,17 @@ public class SocketWrapper<E> { private Set<DispatchType> dispatches = new LinkedHashSet<>(); + /* + * Used to indicate that the socket is in the process of closing / has been + * closed. Once this flag has been set, no further reads or writes should + * take place. Its primary purpose is with upgraded connections where a + * socket may be in use in application code with no immediate way to signal + * that the socket is no longer valid. Checking this flag before any + * application triggered read or write will enable an IOException to be + * thrown. + */ + private boolean closing = false; + public SocketWrapper(E socket) { this.socket = socket; ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); @@ -131,10 +142,13 @@ public class SocketWrapper<E> { public void clearDispatches() { dispatches.clear(); } + public boolean isClosing() { return closing; } + public void setClosing(boolean closing) { this.closing = closing; } public void reset(E socket, long timeout) { async = false; blockingStatus = true; + closing = false; comet = false; dispatches.clear(); error = false; Modified: tomcat/trunk/java/org/apache/tomcat/util/net/res/LocalStrings.properties URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/res/LocalStrings.properties?rev=1523964&r1=1523963&r2=1523964&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/net/res/LocalStrings.properties (original) +++ tomcat/trunk/java/org/apache/tomcat/util/net/res/LocalStrings.properties Tue Sep 17 09:44:32 2013 @@ -22,6 +22,7 @@ endpoint.warn.noDisableCompression='Disa endpoint.warn.noHonorCipherOrder='Honor cipher order' option is not supported by the SSL library {0} endpoint.warn.noInsecureReneg=Secure re-negotiation is not supported by the SSL library {0} endpoint.warn.unlockAcceptorFailed=Acceptor thread [{0}] failed to unlock. Forcing hard socket shutdown. +endpoint.warn.executorShutdown=The executor associated with thread pool [{0}] has not fully shutdown. Some application threads may still be running. endpoint.debug.channelCloseFail=Failed to close channel endpoint.debug.destroySocket=socket [{0}], doIt [{1}] endpoint.debug.pollerAdd=socket [{0}], timeout [{1}], flags [{2}] --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org