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 2013/09/17 11:44:33 UTC

svn commit: r1523964 - in /tomcat/trunk/java/org/apache: coyote/ coyote/http11/ coyote/http11/upgrade/ tomcat/util/net/ tomcat/util/net/res/

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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org