On 05/07/2012 08:48 PM, Mark Thomas wrote:
I'm not entirely clear on what you are proposing. Can you provide a
proposed patch for this?


Sure.
Need to port that to trunk and new AbstractEndpoint.
Attached is a patch for something similar to tomcat7 :)
just to get an idea.


AFAICT this is not covered by any spec and the only
change is that a single connection that could get accepted
during pause phase can receive ECONNRESET instead ECONNREFUSED
in case pause continues beyond client socket timeout.

That will be bad for connections from a reverse proxy when taking a
Tomcat instance out of a cluster. I'm leaning towards objecting to this
patch on that basis alone.

Think not.
Currently acceptor thread waits before calling accept if paused.
The proposed solution will wait after accept returns.
So there are actually two scenarios.
a) Optimistic:
   No connections accepted between pause/start
   This is advantage over current impl, cause no unlock needed.
b) Pessimistic:
   While in pause accept() was fired and new socket created.
   while(pause) { sleep; }
   If stopped destroy socket and quit acceptor.
   If running continue processing accepted socket.

So in the worse case a single connection will get accepted and
sit in the while(paused) { } block.
Note that this is no different then having a backlog of 1 thought.


Regards
--
^TM
Index: java/org/apache/tomcat/util/net/JIoEndpoint.java
===================================================================
--- java/org/apache/tomcat/util/net/JIoEndpoint.java	(revision 2033)
+++ java/org/apache/tomcat/util/net/JIoEndpoint.java	(working copy)
@@ -307,18 +307,23 @@
             // Loop until we receive a shutdown command
             while (running) {
 
-                // Loop if endpoint is paused
-                while (paused) {
-                    try {
-                        Thread.sleep(1000);
-                    } catch (InterruptedException e) {
-                        // Ignore
-                    }
-                }
-
                 // Accept the next incoming connection from the server socket
                 try {
                     Socket socket = serverSocketFactory.acceptSocket(serverSocket);
+                    // Loop if endpoint is paused
+                    while (paused) {
+                        try {
+                            Thread.sleep(100);
+                        } catch (InterruptedException e) {
+                            // Ignore
+                        }
+                    }
+                    if (!running) {
+                        // Transition from pause->stop.
+                        // Close pending socket right away.
+                        try { socket.close(); } catch (IOException e) { }
+                        break;
+                    }
                     serverSocketFactory.initSocket(socket);
                     // Hand this socket off to an appropriate processor
                     if (!processSocket(socket)) {
@@ -726,11 +731,15 @@
                 // Loop if endpoint is paused
                 while (paused) {
                     try {
-                        Thread.sleep(1000);
+                        Thread.sleep(100);
                     } catch (InterruptedException e) {
                         // Ignore
                     }
                 }
+                if (!running) {
+                    // Transition from pause->stop. Break right away.
+                    break;
+                }
                 // Check timeouts for suspended connections if the poller is empty
                 while (connectionCount < 1 && addList.size() < 1) {
                     // Reset maintain time.
@@ -1028,7 +1037,6 @@
     public void pause() {
         if (running && !paused) {
             paused = true;
-            unlockAccept();
         }
     }
 
@@ -1040,8 +1048,18 @@
 
     public void stop() {
         if (running) {
+            paused  = false;
             running = false;
-            unlockAccept();
+            if (serverSocket != null) {
+                try {
+                    if (serverSocket != null)
+                        serverSocket.close();
+                } catch (Exception e) {
+                    log.error(sm.getString("endpoint.err.close"), e);
+                }
+                serverSocket = null;
+            }
+            initialized = false ;
             eventPoller.destroy();
             eventPoller = null;
         }
@@ -1066,39 +1084,7 @@
         initialized = false ;
     }
 
-    
     /**
-     * Unlock the accept by using a local connection.
-     */
-    protected void unlockAccept() {
-        Socket s = null;
-        try {
-            // Need to create a connection to unlock the accept();
-            if (address == null) {
-                s = new Socket("localhost", port);
-            } else {
-                s = new Socket(address, port);
-                    // setting soLinger to a small value will help shutdown the
-                    // connection quicker
-                s.setSoLinger(true, 0);
-            }
-        } catch (Exception e) {
-            if (log.isDebugEnabled()) {
-                log.debug(sm.getString("endpoint.debug.unlock", "" + port), e);
-            }
-        } finally {
-            if (s != null) {
-                try {
-                    s.close();
-                } catch (Exception e) {
-                    // Ignore
-                }
-            }
-        }
-    }
-
-
-    /**
      * Set the options for the current socket.
      */
     protected boolean setSocketOptions(Socket socket) {

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

Reply via email to