Author: markt
Date: Tue Sep 17 09:58:30 2013
New Revision: 1523967

URL: http://svn.apache.org/r1523967
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/tc7.0.x/trunk/   (props changed)
    tomcat/tc7.0.x/trunk/java/org/apache/coyote/AbstractProtocol.java
    tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11AprProtocol.java
    
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/upgrade/AprServletInputStream.java
    
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/upgrade/AprServletOutputStream.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/AprEndpoint.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/SocketWrapper.java
    
tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/res/LocalStrings.properties

Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
  Merged /tomcat/trunk:r1523964

Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/AbstractProtocol.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/AbstractProtocol.java?rev=1523967&r1=1523966&r2=1523967&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/coyote/AbstractProtocol.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/AbstractProtocol.java Tue Sep 
17 09:58:30 2013
@@ -675,6 +675,7 @@ public abstract class AbstractProtocol i
                 } 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/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11AprProtocol.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11AprProtocol.java?rev=1523967&r1=1523966&r2=1523967&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11AprProtocol.java 
(original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11AprProtocol.java 
Tue Sep 17 09:58:30 2013
@@ -27,6 +27,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;
 
@@ -279,8 +280,13 @@ public class Http11AprProtocol extends A
                 }
             } else if (processor.isUpgrade()) {
                 // 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);
+                }
             } else {
                 // Tomcat 7 proprietary upgrade
                 ((AprEndpoint) proto.endpoint).getPoller().add(

Modified: 
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/upgrade/AprServletInputStream.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/upgrade/AprServletInputStream.java?rev=1523967&r1=1523966&r2=1523967&view=diff
==============================================================================
--- 
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/upgrade/AprServletInputStream.java
 (original)
+++ 
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/upgrade/AprServletInputStream.java
 Tue Sep 17 09:58:30 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/tc7.0.x/trunk/java/org/apache/coyote/http11/upgrade/AprServletOutputStream.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/upgrade/AprServletOutputStream.java?rev=1523967&r1=1523966&r2=1523967&view=diff
==============================================================================
--- 
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/upgrade/AprServletOutputStream.java
 (original)
+++ 
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/upgrade/AprServletOutputStream.java
 Tue Sep 17 09:58:30 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/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=1523967&r1=1523966&r2=1523967&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 
Tue Sep 17 09:58:30 2013
@@ -504,6 +504,18 @@ public abstract class AbstractEndpoint {
                 //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/tc7.0.x/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java?rev=1523967&r1=1523966&r2=1523967&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java 
(original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java Tue 
Sep 17 09:58:30 2013
@@ -1585,6 +1585,10 @@ public class AprEndpoint extends Abstrac
                     }
                 }
 
+                if (!pollerRunning) {
+                    break;
+                }
+
                 try {
                     // Add sockets which are waiting to the poller
                     if (addList.size() > 0) {

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=1523967&r1=1523966&r2=1523967&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 Tue 
Sep 17 09:58:30 2013
@@ -1511,13 +1511,12 @@ public class NioEndpoint extends Abstrac
         }
 
         public void reset(Poller poller, NioChannel channel, long soTimeout) {
-            this.socket = channel;
+            super.reset(channel, soTimeout);
+
+            cometNotify = false;
+            cometOps = SelectionKey.OP_READ;
+            interestOps = 0;
             this.poller = poller;
-            lastAccess = System.currentTimeMillis();
-            setComet(false);
-            timeout = soTimeout;
-            setWriteTimeout(soTimeout);
-            error = false;
             lastRegistered = 0;
             sendfileData = null;
             if (readLatch != null) {
@@ -1529,6 +1528,7 @@ public class NioEndpoint extends Abstrac
                 }
             }
             readLatch = null;
+            sendfileData = null;
             if (writeLatch != null) {
                 try {
                     for (int i = 0; i < (int) writeLatch.getCount(); i++) {
@@ -1538,11 +1538,7 @@ public class NioEndpoint extends Abstrac
                 }
             }
             writeLatch = null;
-            cometNotify = false;
-            cometOps = SelectionKey.OP_READ;
-            sendfileData = null;
-            keepAliveLeft = 100;
-            async = false;
+            setWriteTimeout(soTimeout);
         }
 
         public void reset() {

Modified: 
tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/SocketWrapper.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/SocketWrapper.java?rev=1523967&r1=1523966&r2=1523967&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/SocketWrapper.java 
(original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/SocketWrapper.java Tue 
Sep 17 09:58:30 2013
@@ -52,6 +52,17 @@ public class SocketWrapper<E> {
      */
     private final Object writeThreadLock = new Object();
 
+    /*
+     * 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();
@@ -89,4 +100,19 @@ public class SocketWrapper<E> {
         return blockingStatusWriteLock;
     }
     public Object getWriteThreadLock() { return writeThreadLock; }
+    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;
+        error = false;
+        keepAliveLeft = 100;
+        lastAccess = System.currentTimeMillis();
+        this.socket = socket;
+        this.timeout = timeout;
+        upgraded = false;
+    }
 }

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=1523967&r1=1523966&r2=1523967&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
 Tue Sep 17 09:58:30 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

Reply via email to