tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kkoli...@apache.org
Subject svn commit: r1065945 - in /tomcat/tc6.0.x/trunk: STATUS.txt java/org/apache/tomcat/util/net/AprEndpoint.java webapps/docs/changelog.xml
Date Tue, 01 Feb 2011 08:07:46 GMT
Author: kkolinko
Date: Tue Feb  1 08:07:46 2011
New Revision: 1065945

URL: http://svn.apache.org/viewvc?rev=1065945&view=rev
Log:
Backport AprEndpoint shutdown patch (BZ 49795 and similar).

Modified:
    tomcat/tc6.0.x/trunk/STATUS.txt
    tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java
    tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml

Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1065945&r1=1065944&r2=1065945&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Tue Feb  1 08:07:46 2011
@@ -89,11 +89,6 @@ PATCHES PROPOSED TO BACKPORT:
               We can stall this item until we get some feedback about 7.0.5.
   -1:
 
-* Backport AprEndpoint shutdown patch (BZ49795 and similar).
-  http://people.apache.org/~kkolinko/patches/2011-01-24_tc6_aprshutdown.patch
-  +1: mturk, kkolinko, markt
-  -1:
-
 * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=50642
   Better memory leak protection for HttpClient keep-alive thread
   http://svn.apache.org/viewvc?rev=1064652&view=rev

Modified: tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java?rev=1065945&r1=1065944&r2=1065945&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java Tue Feb  1 08:07:46
2011
@@ -157,6 +157,8 @@ public class AprEndpoint {
      */
     protected long sslContext = 0;
 
+    /* Acceptor thread array */
+    private Acceptor acceptors[] = null;
 
     // ------------------------------------------------------------- Properties
 
@@ -767,10 +769,10 @@ public class AprEndpoint {
             for (int i = 0; i < pollerThreadCount; i++) {
                 pollers[i] = new Poller(false);
                 pollers[i].init();
-                Thread pollerThread = new Thread(pollers[i], getName() + "-Poller-" + i);
-                pollerThread.setPriority(threadPriority);
-                pollerThread.setDaemon(true);
-                pollerThread.start();
+                pollers[i].setName(getName() + "-Poller-" + i);
+                pollers[i].setPriority(threadPriority);
+                pollers[i].setDaemon(true);
+                pollers[i].start();
             }
 
             // Start comet poller threads
@@ -778,10 +780,10 @@ public class AprEndpoint {
             for (int i = 0; i < pollerThreadCount; i++) {
                 cometPollers[i] = new Poller(true);
                 cometPollers[i].init();
-                Thread pollerThread = new Thread(cometPollers[i], getName() + "-CometPoller-"
+ i);
-                pollerThread.setPriority(threadPriority);
-                pollerThread.setDaemon(true);
-                pollerThread.start();
+                cometPollers[i].setName(getName() + "-CometPoller-" + i);
+                cometPollers[i].setPriority(threadPriority);
+                cometPollers[i].setDaemon(true);
+                cometPollers[i].start();
             }
 
             // Start sendfile threads
@@ -790,19 +792,21 @@ public class AprEndpoint {
                 for (int i = 0; i < sendfileThreadCount; i++) {
                     sendfiles[i] = new Sendfile();
                     sendfiles[i].init();
-                    Thread sendfileThread = new Thread(sendfiles[i], getName() + "-Sendfile-"
+ i);
-                    sendfileThread.setPriority(threadPriority);
-                    sendfileThread.setDaemon(true);
-                    sendfileThread.start();
+                    sendfiles[i].setName(getName() + "-Sendfile-" + i);
+                    sendfiles[i].setPriority(threadPriority);
+                    sendfiles[i].setDaemon(true);
+                    sendfiles[i].start();
                 }
             }
 
             // Start acceptor threads
+            acceptors = new Acceptor[acceptorThreadCount];
             for (int i = 0; i < acceptorThreadCount; i++) {
-                Thread acceptorThread = new Thread(new Acceptor(), getName() + "-Acceptor-"
+ i);
-                acceptorThread.setPriority(threadPriority);
-                acceptorThread.setDaemon(daemon);
-                acceptorThread.start();
+                acceptors[i] = new Acceptor();
+                acceptors[i].setName(getName() + "-Acceptor-" + i);
+                acceptors[i].setPriority(threadPriority);
+                acceptors[i].setDaemon(getDaemon());
+                acceptors[i].start();
             }
 
         }
@@ -835,20 +839,56 @@ public class AprEndpoint {
      * Stop the endpoint. This will cause all processing threads to stop.
      */
     public void stop() {
+        if (!paused) {
+            pause();
+        }
         if (running) {
             running = false;
             unlockAccept();
+            for (int i = 0; i < acceptors.length; i++) {
+                long s = System.currentTimeMillis() + 30000;
+                while (acceptors[i].isAlive()) {
+                    try {
+                        acceptors[i].interrupt();
+                        acceptors[i].join(1000);
+                    } catch (InterruptedException e) {
+                        // Ignore
+                    }
+                    if (System.currentTimeMillis() >= s) {
+                        log.warn(sm.getString("endpoint.warn.unlockAcceptorFailed",
+                                 acceptors[i].getName()));
+                        // If the Acceptor is still running force
+                        // the hard socket close.
+                        if (serverSock != 0) {
+                            Socket.shutdown(serverSock, Socket.APR_SHUTDOWN_READ);
+                            serverSock = 0;
+                        }
+                    }
+                }
+            }
             for (int i = 0; i < pollers.length; i++) {
-                pollers[i].destroy();
+                try {
+                    pollers[i].destroy();
+                } catch (Exception e) {
+                    // Ignore
+                }
             }
             pollers = null;
             for (int i = 0; i < cometPollers.length; i++) {
-                cometPollers[i].destroy();
+                try {
+                    cometPollers[i].destroy();
+                } catch (Exception e) {
+                    // Ignore
+                }
             }
             cometPollers = null;
             if (useSendfile) {
                 for (int i = 0; i < sendfiles.length; i++) {
-                    sendfiles[i].destroy();
+                    try {
+                        sendfiles[i].destroy();
+                    } catch (Exception e) {
+                        // Ignore
+                    }
                 }
                 sendfiles = null;
             }
@@ -863,15 +903,26 @@ public class AprEndpoint {
         if (running) {
             stop();
         }
-        Pool.destroy(serverSockPool);
-        serverSockPool = 0;
-        // Close server socket
-        Socket.close(serverSock);
-        serverSock = 0;
+        // Destroy pool if it was initialised
+        if (serverSockPool != 0) {
+            Pool.destroy(serverSockPool);
+            serverSockPool = 0;
+        }
+
+        // Close server socket if it was initialised
+        if (serverSock != 0) {
+            Socket.close(serverSock);
+            serverSock = 0;
+        }
+
         sslContext = 0;
-        // Close all APR memory pools and resources
-        Pool.destroy(rootPool);
-        rootPool = 0;
+
+        // Close all APR memory pools and resources if initialised
+        if (rootPool != 0) {
+            Pool.destroy(rootPool);
+            rootPool = 0;
+        }
+
         initialized = false;
     }
 
@@ -1136,6 +1187,15 @@ public class AprEndpoint {
         return true;
     }
 
+    private void destroySocket(long socket) {
+        if (running && socket != 0) {
+            // If not running the socket will be destroyed by
+            // parent pool or acceptor socket.
+            // In any case disable double free which would cause JVM core.
+            Socket.destroy(socket);
+        }
+    }
+
 
     // --------------------------------------------------- Acceptor Inner Class
 
@@ -1143,7 +1203,7 @@ public class AprEndpoint {
     /**
      * Server socket acceptor thread.
      */
-    protected class Acceptor implements Runnable {
+    protected class Acceptor extends Thread {
 
         private final Log log = LogFactory.getLog(AprEndpoint.Acceptor.class);
 
@@ -1157,7 +1217,7 @@ public class AprEndpoint {
             while (running) {
 
                 // Loop if endpoint is paused
-                while (paused) {
+                while (paused && running) {
                     try {
                         Thread.sleep(1000);
                     } catch (InterruptedException e) {
@@ -1165,6 +1225,9 @@ public class AprEndpoint {
                     }
                 }
 
+                if (!running) {
+                    break;
+                }
                 try {
                     // Accept the next incoming connection from the server socket
                     long socket = Socket.accept(serverSock);
@@ -1174,13 +1237,13 @@ public class AprEndpoint {
                      * socket and don't process it.
                      */
                     if (deferAccept && (paused || !running)) {
-                        Socket.destroy(socket);
+                        destroySocket(socket);
                         continue;
                     }
                     // Hand this socket off to an appropriate processor
                     if (!processSocketWithOptions(socket)) {
                         // Close socket and pool right away
-                        Socket.destroy(socket);
+                        destroySocket(socket);
                     }
                 } catch (Throwable t) {
                     if (running) {
@@ -1216,7 +1279,7 @@ public class AprEndpoint {
     /**
      * Poller class.
      */
-    public class Poller implements Runnable {
+    public class Poller extends Thread {
 
         protected long serverPollset = 0;
         protected long pool = 0;
@@ -1263,23 +1326,13 @@ public class AprEndpoint {
         /**
          * Destroy the poller.
          */
-        protected void destroy() {
-            // Wait for polltime before doing anything, so that the poller threads
-            // exit, otherwise parallel descturction of sockets which are still
-            // in the poller can cause problems
-            try {
-                synchronized (this) {
-                    this.wait(pollTime / 1000);
-                }
-            } catch (InterruptedException e) {
-                // Ignore
-            }
+        public void destroy() {
             // Close all sockets in the add queue
             for (int i = 0; i < addCount; i++) {
                 if (comet) {
                     processSocket(addS[i], SocketStatus.STOP);
                 } else {
-                    Socket.destroy(addS[i]);
+                    destroySocket(addS[i]);
                 }
             }
             // Close all sockets still in the poller
@@ -1289,13 +1342,21 @@ public class AprEndpoint {
                     if (comet) {
                         processSocket(desc[n*2+1], SocketStatus.STOP);
                     } else {
-                        Socket.destroy(desc[n*2+1]);
+                        destroySocket(desc[n*2+1]);
                     }
                 }
             }
             Pool.destroy(pool);
             keepAliveCount = 0;
             addCount = 0;
+            try {
+                while (this.isAlive()) {
+                    this.interrupt();
+                    this.join(1000);
+                }
+            } catch (InterruptedException e) {
+                // Ignore
+            }
         }
 
         /**
@@ -1315,7 +1376,7 @@ public class AprEndpoint {
                     if (comet) {
                         processSocket(socket, SocketStatus.ERROR);
                     } else {
-                        Socket.destroy(socket);
+                        destroySocket(socket);
                     }
                     return;
                 }
@@ -1335,7 +1396,7 @@ public class AprEndpoint {
             // Loop until we receive a shutdown command
             while (running) {
                 // Loop if endpoint is paused
-                while (paused) {
+                while (paused && running) {
                     try {
                         Thread.sleep(1000);
                     } catch (InterruptedException e) {
@@ -1343,9 +1404,12 @@ public class AprEndpoint {
                     }
                 }
 
+                if (!running) {
+                    break;
+                }
                 if (keepAliveCount < 1 && addCount < 1) {
                     synchronized (this) {
-                        while (keepAliveCount < 1 && addCount < 1) {
+                        while (keepAliveCount < 1 && addCount < 1 &&
running) {
                             // Reset maintain time.
                             maintainTime = 0;
                             try {
@@ -1357,6 +1421,9 @@ public class AprEndpoint {
                     }
                 }
 
+                if (!running) {
+                    break;
+                }
                 try {
                     // Add sockets which are waiting to the poller
                     if (addCount > 0) {
@@ -1373,7 +1440,7 @@ public class AprEndpoint {
                                         if (comet) {
                                             processSocket(addS[i], SocketStatus.ERROR);
                                         } else {
-                                            Socket.destroy(addS[i]);
+                                            destroySocket(addS[i]);
                                         }
                                     }
                                 }
@@ -1399,7 +1466,7 @@ public class AprEndpoint {
                                 if (comet) {
                                     processSocket(desc[n*2+1], SocketStatus.DISCONNECT);
                                 } else {
-                                    Socket.destroy(desc[n*2+1]);
+                                    destroySocket(desc[n*2+1]);
                                 }
                                 continue;
                             }
@@ -1430,7 +1497,7 @@ public class AprEndpoint {
                                 if (comet) {
                                     processSocket(desc[n], SocketStatus.TIMEOUT);
                                 } else {
-                                    Socket.destroy(desc[n]);
+                                    destroySocket(desc[n]);
                                 }
                             }
                         }
@@ -1444,9 +1511,7 @@ public class AprEndpoint {
             synchronized (this) {
                 this.notifyAll();
             }
-
         }
-
     }
 
 
@@ -1587,7 +1652,7 @@ public class AprEndpoint {
                         getPoller().add(socket);
                     } else {
                         // Close socket and pool
-                        Socket.destroy(socket);
+                        destroySocket(socket);
                         socket = 0;
                     }
                 } else {
@@ -1595,12 +1660,12 @@ public class AprEndpoint {
                     // Process the request from this socket
                     if ((status != null) && (handler.event(socket, status) == Handler.SocketState.CLOSED))
{
                         // Close socket and pool
-                        Socket.destroy(socket);
+                        destroySocket(socket);
                         socket = 0;
                     } else if ((status == null) && ((options && !setSocketOptions(socket))
                             || handler.process(socket) == Handler.SocketState.CLOSED)) {
                         // Close socket and pool
-                        Socket.destroy(socket);
+                        destroySocket(socket);
                         socket = 0;
                     }
                 }
@@ -1656,7 +1721,7 @@ public class AprEndpoint {
     /**
      * Sendfile class.
      */
-    public class Sendfile implements Runnable {
+    public class Sendfile extends Thread {
 
         protected long sendfilePollset = 0;
         protected long pool = 0;
@@ -1694,32 +1759,31 @@ public class AprEndpoint {
         /**
          * Destroy the poller.
          */
-        protected void destroy() {
-            // Wait for polltime before doing anything, so that the poller threads
-            // exit, otherwise parallel descturction of sockets which are still
-            // in the poller can cause problems
-            try {
-                synchronized (this) {
-                    this.wait(pollTime / 1000);
-                }
-            } catch (InterruptedException e) {
-                // Ignore
-            }
+        public void destroy() {
             // Close any socket remaining in the add queue
             addCount = 0;
             for (int i = (addS.size() - 1); i >= 0; i--) {
                 SendfileData data = addS.get(i);
-                Socket.destroy(data.socket);
+                destroySocket(data.socket);
             }
+            addS.clear();
             // Close all sockets still in the poller
             int rv = Poll.pollset(sendfilePollset, desc);
             if (rv > 0) {
                 for (int n = 0; n < rv; n++) {
-                    Socket.destroy(desc[n*2+1]);
+                    destroySocket(desc[n*2+1]);
                 }
             }
             Pool.destroy(pool);
             sendfileData.clear();
+            try {
+                while (this.isAlive()) {
+                    this.interrupt();
+                    this.join(1000);
+                }
+            } catch (InterruptedException e) {
+                // Ignore
+            }
         }
 
         /**
@@ -1748,7 +1812,7 @@ public class AprEndpoint {
                                                data.pos, data.end - data.pos, 0);
                     if (nw < 0) {
                         if (!(-nw == Status.EAGAIN)) {
-                            Socket.destroy(data.socket);
+                            destroySocket(data.socket);
                             data.socket = 0;
                             return false;
                         } else {
@@ -1804,7 +1868,7 @@ public class AprEndpoint {
             while (running) {
 
                 // Loop if endpoint is paused
-                while (paused) {
+                while (paused && running) {
                     try {
                         Thread.sleep(1000);
                     } catch (InterruptedException e) {
@@ -1812,9 +1876,12 @@ public class AprEndpoint {
                     }
                 }
 
+                if (!running) {
+                    break;
+                }
                 if (sendfileCount < 1 && addCount < 1) {
                     synchronized (this) {
-                        while (sendfileCount < 1 && addS.size() < 1) {
+                        while (sendfileCount < 1 && addS.size() < 1 &&
running) {
                             // Reset maintain time.
                             maintainTime = 0;
                             try {
@@ -1826,6 +1893,9 @@ public class AprEndpoint {
                     }
                 }
 
+                if (!running) {
+                    break;
+                }
                 try {
                     // Add socket to the poller
                     if (addCount > 0) {
@@ -1841,7 +1911,7 @@ public class AprEndpoint {
                                     } else {
                                         log.warn(sm.getString("endpoint.sendfile.addfail",
"" + rv, Error.strerror(rv)));
                                         // Can't do anything: close the socket right away
-                                        Socket.destroy(data.socket);
+                                        destroySocket(data.socket);
                                     }
                                 }
                             } finally {
@@ -1867,7 +1937,7 @@ public class AprEndpoint {
                                 remove(state);
                                 // Destroy file descriptor pool, which should close the file
                                 // Close the socket, as the reponse would be incomplete
-                                Socket.destroy(state.socket);
+                                destroySocket(state.socket);
                                 continue;
                             }
                             // Write some data using sendfile
@@ -1879,7 +1949,7 @@ public class AprEndpoint {
                                 remove(state);
                                 // Close the socket, as the reponse would be incomplete
                                 // This will close the file too.
-                                Socket.destroy(state.socket);
+                                destroySocket(state.socket);
                                 continue;
                             }
 
@@ -1896,7 +1966,7 @@ public class AprEndpoint {
                                 } else {
                                     // Close the socket since this is
                                     // the end of not keep-alive request.
-                                    Socket.destroy(state.socket);
+                                    destroySocket(state.socket);
                                 }
                             }
                         }
@@ -1928,7 +1998,7 @@ public class AprEndpoint {
                                 remove(state);
                                 // Destroy file descriptor pool, which should close the file
                                 // Close the socket, as the response would be incomplete
-                                Socket.destroy(state.socket);
+                                destroySocket(state.socket);
                             }
                         }
                     }
@@ -2064,7 +2134,7 @@ public class AprEndpoint {
                     getPoller().add(socket);
                 } else {
                     // Close socket and pool
-                    Socket.destroy(socket);
+                    destroySocket(socket);
                     socket = 0;
                 }
             } else {
@@ -2072,7 +2142,7 @@ public class AprEndpoint {
                 if (!setSocketOptions(socket)
                         || handler.process(socket) == Handler.SocketState.CLOSED) {
                     // Close socket and pool
-                    Socket.destroy(socket);
+                    destroySocket(socket);
                     socket = 0;
                 }
             }
@@ -2102,7 +2172,7 @@ public class AprEndpoint {
             // Process the request from this socket
             if (handler.process(socket) == Handler.SocketState.CLOSED) {
                 // Close socket and pool
-                Socket.destroy(socket);
+                destroySocket(socket);
                 socket = 0;
             }
 
@@ -2133,7 +2203,7 @@ public class AprEndpoint {
             // Process the request from this socket
             if (handler.event(socket, status) == Handler.SocketState.CLOSED) {
                 // Close socket and pool
-                Socket.destroy(socket);
+                destroySocket(socket);
                 socket = 0;
             }
 

Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=1065945&r1=1065944&r2=1065945&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Tue Feb  1 08:07:46 2011
@@ -55,6 +55,10 @@
   <subsection name="Coyote">
     <changelog>
       <fix>
+        <bug>49795</bug>: Backport AprEndpoint shutdown improvements, to make
+        it more robust. (mturk/kkolinko)
+      </fix>
+      <fix>
         <bug>50651</bug>: Fix NPE in InternalNioOutputBuffer.recycle().
         (kkolinko)
       </fix>



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message